Bricksmith crashing because GL_VERTEXBUFFER, ... is not enabled

[d029940]

Im on MacOS Mojave and compiling with Xcode 10

I am getting a SIGART at bricksmith_bsupnik/trunk/Bricksmith/Source/LDraw/Support/LDrawGLRenderer.m, line 232:
assert(glIsEnabled(GL_VERTEX_ARRAY));

It seems to be that GL_VERTEX_BUFFER was not enabled.
This is also true for:
assert(glIsEnabled(GL_NORMAL_ARRAY));
assert(glIsEnabled(GL_COLOR_ARRAY));

I put in LDrawGLView.m - internalInit - the following statements, just after
// CGLEnable(CGLGetCurrentContext(), kCGLCEMPEngine);

glEnableClientState(GL_VERTEX_ARRAY);
glEnableClientState(GL_NORMAL_ARRAY);
glEnableClientState(GL_COLOR_ARRAY);

I am not sure what I am doing - I am not an OpenGL expert. But with these 3 statements, Bricksmith starts, but the windows managed by OpenGL are gray.

Are you using Xcode 9?

[bsupnik]

Hi, I have Bricksmith running on Mojave and older OSes, but if it is compiled with Mojave against the 10.14 SDK, something goes wrong in view-clipping and I can't see anything. When compliing via X-code 8, the app works fine. I'll try to post a beta shortly.

…

On Mon, Jan 21, 2019 at 4:28 AM d029940 ***@***.***> wrote: Im on MacOS Mojave and compiling with Xcode 10 I am getting a SIGART at bricksmith_bsupnik/trunk/Bricksmith/Source/LDraw/Support/LDrawGLRenderer.m, line 232: assert(glIsEnabled(GL_VERTEX_ARRAY)); It seems to be that GL_VERTEX_BUFFER was not enabled. This is also true for: assert(glIsEnabled(GL_NORMAL_ARRAY)); assert(glIsEnabled(GL_COLOR_ARRAY)); I put in LDrawGLView.m - internalInit - the following statements, just after // CGLEnable(CGLGetCurrentContext(), kCGLCEMPEngine); glEnableClientState(GL_VERTEX_ARRAY); glEnableClientState(GL_NORMAL_ARRAY); glEnableClientState(GL_COLOR_ARRAY); I am not sure what I am doing - I am not an OpenGL expert. But with these 3 statements, Bricksmith starts, but the windows managed by OpenGL are gray. Are you using Xcode 9? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABqPH4g2PxBxmZoip0CXi3x8BzcrpUYBks5vFYghgaJpZM4aKaos> .

The reason for the error is that the LDrawGLRenderer is nil at this point and thus, prepareOpenGL can not be called. It works if the initialization of the LDrawGLRenderer is moved before the creation of the context in LDrawGLView.m

Is this project still maintained? I could create a pull request with the modification.

[bsupnik]

@wayfaerer I have not seen this crash in current code. What's the GIT SHA where you see this and where your patch would apply? I'll look at whether the case is fixed in the latest code, then either merge that to master or take a pull.

@bsupnik I checked out the master, the sha is 9eaa764.
The fix would move the lines 239 - 241 in LDrawGLView before line 211 where the NSOpenGLContext is initalized.
The base sdk I'm using is 10.15.

[d029940]

I just cloned the master.
Opening the project file (Bricksmith.xcodeproj) in trunk, I am getting all sorts of warnings (SDK points to 10.12, 32-bit Architecture, ...), code signing error for ldsynth ...

And as far as I know, no one succeeded up to now using the sdk 10.15. It must be an older one because of the "layered Open-GL-View)

Am I doing something wrong? Am I correct?

Strange, I'm pretty sure I also have the project file from the trunk in the master.
git log --pretty=format:'%h' -n 1 says: 9eaa764

I also got this code signing warning. It went away after I added "--deep" to "Other Code Signing Flags" for the Bricksmith Target.

[d029940]

First of all, thank you. Adding "--deep" to "Other Code Signing Flags" helped. Also with moving the lines 239 - 241 in LDrawGLView before line 211 where the NSOpenGLContext is initalized, there is no runtime error anymore.
But we are back at the start. On loading a model, the model is shortly flushed on the GLView and then disappears. (Building on Catalina, XCode 11.4.1 and most up to date sdk.

Yes, you will need one other fix. Add the Line
[self setBackgroundColor:[NSColor clearColor]];
in initWithContentRect: of the class OverlayHelperWindow (line 68)

With this, the model should be visible, although it is streched vetically.

[d029940]

Great. Thank you. That did the trick. The model is visible (and stretched).
Is there a way to have a branch with all these litte modifications?
So, that we have a change to clean the project file (no more 32 bit, ...)

I think, in order that bricksmith will survive, we need to convert to modern Obj-C and in the very end I think we need to move to Metal.

Two years ago I started with getting rid of retain/release for some modules, but was stopped by the OpenGL problem wit Catalina.
I also had a look into Metal. But beyond some easy tutorials, it is really very difficult - and at least if you have no knowledge about OpenGL and only limited knowledge about 3D.

Basically that should not be a problem. @bsupnik did you already have the time to look into the latest code? What branch is this on?

@d029940 I thought exactly the same, bringing the project to ARC and eventually to Metal will be necessary for Bricksmith to survive. Unfortunately, I also don't know much about OpenGL and Metal. There's a Talk at WWDC for such a migration by the way. (https://developer.apple.com/videos/play/wwdc2019/611/)

It would really be a shame if Bricksmith dies, it's a very good editor for LDraw. I think I would have some time to spare for this project, but I think it's a lot of work to be done overall.

[bsupnik]

Hi Y'all,

I'm running on e5cfed1. Note that you need an old 10.12 SDK to build this.

This is the only way I have found to have zoom work on Mojave and Catalina. In older GL-based apps, the content view doesn't have its own layer, and therefore when it "zooms" the pixel density doesn't change. This changed recently - it's as if "has its own layer" is always forced on, and the result is that BrickSmith's zooming strategy completely breaks down. In the long term, we clearly need new zoom code that doesn't depend on the old GL-borrows-a-layer trick, it was just beyond the scope of what I had time for to make that mod.

This branch compiles, links, and runs correctly when built on Mojave; when built on Catalina, you have to use a linker flag to hack around a missing symbol and then the zoom code still ends up buggy - zooming in shows pixelation.

So...I think this is my thinking:

• Short term: I can merge e5cfed1 to master and cut a build off of it on my Mojave machine - this build will work well on Mojave and Catalina and gets everyone running with the latest bug fixes and features I've coded, and no bit rot from tech debt.
• Short term: the project is pretty unusable on Catalina - if someone wants to make make a pull request to make e5cfed1 work on Catalina (linker fix, maybe remove the SDK hack, any other small bugs like init order, etc.) I can take them either to master if they don't break Mojave builds or Catalina if they do.
• Medium term: the thing the project needs most is rewritten code for zoom & scroll, which is broken on all modern SDKs and any Catalina dev machine - it's only a matter of time before we're just toast. Having this would also let us have a unified code base that runs on a bunch of modern Mac OSes.
• Long term: migrate to arc and metal. Both are nice to have - Metal is in a really nice spot between OpenGL and Vulkan in terms of perf and complexity. But I think both of these things pose less of an existential threat to Bricksmith working than solving the view problems and other bugs introduced by perpetual AppKit changes. I've been helping Allen maintain Bricksmith for a while and poking it at myself, and the zoom weirdness on Catalina isn't a one-off occurrence; there's just been a steady stream of this kind of thing.

AppKit is very mature and complex and internal new features do things like change callback orders in ways that break old BrickSmith code. So UI is the tech debt I would like to see paid off first. It's also the thing I'm least qualified to deal with, as I'm not an AppKit/ObjC dev by trade.

I am less worried about Apple EOLing OpenGL - there's a whole ecosystem of pro CAD and authoring apps that content creators use on OS X that depend on it, so if I had to put my money down, it would be on Apple leaving it as is and simply never updating it.

This all sounds very reasonable! I checked out e5cfed1 and compiled it with Base SDK 10.15 (deleting derived data before) and it compiled with the „—deep“ code signing flag. I changed the Architecture to Standard to drop 32 bit code.
Also, I still need to set the background to clearColor in the OverlayHelperWindow.
The assertion error with the Open GL Flags is gone in e5cfed1.

This is what it looks now:

There is no distortion in the 3D View anymore, but still in the front and side views.

I only see pixels with zooming when I go to a very high zoom level, like 500%:

Is this what you are refering to?

Unfortunately I have no experience with OpenGL and not much with AppKit, I am quite experienced with IOS development and still familiar with Objective C, might need some time to get back into manual reference counting, though…

What I did not run into are the linker flag and the issue with the init order, but I also didn’t test much.

What about also setting the deployment target to something like at least 10.12 and go after the deprecations? Should also get rid of some old ballast and would be something I could have a look at for a start.

[bsupnik]

@wayfaerer this is exactly what I expect -- distortion in the views and you can see the GL pixels when you move in.

• The first thing is caused by a mismatch of the visible rectangle vs the total rectangle of the "document" that is the GL screen - they can be out of sync! Since the old code rendered to the window (and not the virtual scrolling canvas) we would align to the visible rect. Now that we get our own private canvas, the total rect takes effect.

It is possible to "fix" this by changing the rect we use, but doing so breaks Mojave builds where the old SDK is enough to get the old CALayer behavior. I haven't bothered to check in a "Catalina" version because the second problem is still broken.

• As you zoom, the pixels get bigger and smaller, and this is actually much worse than you'd think. If you make a really, really, really big model (think: whole lego town) the GL canvas size becomes huge to cover the whole span. Since zooming does not change GL pixel size, we end up with either a huge document that kills the graphics card (by drawing the entire model at like 100,000 x 100,000 scale) or really blurry close views (if the whole document is 1000 x 1000).

In other words, we've lost the ability of the pixel density (in terms of bricks) to change with zoom.

ANYWAY...if you'd like to make a topic branch off e5cfed1 to check in some Catalina fixes (no 32-bits, modern SDK, any other fixes), go for it and I'll take a pull request.

In terms of code cleanup, I think my main concern is:

• Clearly there's a bunch of "Apple"-related code cleanup - fixing deprecated APIs, fixing squawks in the xibs, ARC, etc.
• There's also a bunch of "Allen"-related cleanup - current Bricksmith runs based on a rewritten renderer that I hacked out a few years ago to improve drawing speed. But there's still a fair amount of vestigial unused stuff that I haven't cut out yet.
  So if you attempt to clean Apple issues on a usable but unshippable Catalina branch (due to view port issues) while I cut out Allen's dead code on the shipping branch (which requires an old dev machine like I have lying around), do we get merge conflicts later.

I could probably commit to not doing that refactoring any time soon -- the truth is, I've had about zero time to work on BrickSmith and it'd be wasteful to ask you to wait for cleanup that I might do if I can't say when I'm going to get to it.

But in the end, the Catalina view issue is probably the most important one - once that's fixed, wec can unify and everything becomes easier again.

It surly makes total sense to start with the Open-GL issues! Unfortunately this looks like beeing far out of reach for me to give it a try.