#166 - support non-opengl 2D mode

[wijnen]

Here's the pull request for #166 as you requested. The patch applied to the latest version, so that was easy. I originally wrote the "canvas" needed to be a div; that's no longer true. This version expects it to be a canvas, making it easier to select the mode from javascript without needing any changes in the html.

[wijnen]

I'm preparing a new version for you to review. Do you want me to squash the commits so it remains a single commit to m.grl, or should I keep the history as separate commits?

[wijnen]

Never mind; it's too much trouble merging without squashing, so one commit will have to do.

[wijnen]

Ok, I've updated the patch. It's quite a bit longer now. Here's what I did:

• Possibly very important, because I'm not sure if it breaks gl rendering: I moved node.shader.world_matrix to node.world_matrix, because I needed it in 2-D mode, but node.shader is not defined there.
• Add m.dom.js containing 2-D specific parts.
• Add please.renderer which is please.dom or please.gl, depending on which was initialized. It is an object of mostly function pointers for doing renderer-specific operations. They are called from everywhere.
• Add a function sync() to please.SceneGraph, which should be called for 2-D operations instead of draw(). The draw() function calls it before doing a lot of gl things.
• Move some global picker code into a function that is only called for the gl renderer.

Things that probably still need to be done (I didn't test, but didn't implement them):

• Scaling of image nodes.
• Gani animations; or do they work automatically?
• Z stacking. I'm not sure about the best way to do this. The z-index CSS property is ignored for position:absolute elements.

[Aeva]

Getting close! I need to find some time to review more in depth, but the one thing that stands out at the moment is that you pulled a number of areas out of files like m.graph.js and m.camera.js and put them into m.gl.js. I would say that pulling that functionality out into its own method like you did is the right call, but it should still live in the same file. Eg, code that pertains to the camera should live in m.camera.js. m.gl.js exists entirely to provide dynamic bindings to shaders and abstract the crunchier parts of webgl like VBOs.

If you like, I can help clean up this feature (I think it'll mostly just be minor changes) so that we can merge it expediently, but I'd need commit access to your fork.

[Aeva]

also, moving the world_matrix stuff out of node.shader will likely break a lot of things. I'm fine the "shader" namespace on graph nodes still being a thing (sans complicated glsl bindings of course) in non-gl mode, and leaving a note somewhere explaining the misnomer.

[wijnen]

Of course you may help. You can make any changes after you merge it anyway. ;-) I've added you to my fork, so you should be able to push to it.

I'm fine with putting no function implementations in the please.renderer definitions; they are part of both the file they are used in and the renderer implementation, so they make sense in either file.

I considered making an almost empty shader object for the dom renderer. It seems to me like the shader is really a gl thing, so it's out of place there, but it's not a big deal. Also, I think performance should be a higher priority, so if using a shader object helps for that, that's certainly a good reason to go that way.

[Aeva]

Being able to work on the PR will be helpful because I can use github to isolate easily on what has been currently changed, so that way when I pull this into master, it shouldn't break anything.

The node.shader thing serves two purposes, the first being to provide a convenient namespace for data bindings that the developer will probably not need to use frequently, and the second being to provide a place for automatic data bindings to glsl uniform vars, which are them selves data bindings. The world_matrix is there not as a client side convinience, but because glsl needs it to do anything, and it corresponds to a shader binding of the same name. The reason why "location", "rotation", and "scale" are not in this namespace is because they are not uploaded to the GPU (they are only used to generate the world matrix) and because it is expected that the end user will interact with them frequently.

At this point, I'm willing to accept something called "shader" being where utility data bindings are defined as a misnomer, because none of this code was ever intended to run without gl, and so is very tightly coupled to it. Additionally, while other rendering targets are ok (indeed, keeping things modular could be important for future proofing, should something like Vulcan be a viable option), WebGL is the first class rendering target for mgrl.

All that said, I am vaguely considering splitting the 'shader' namespace into 'shader' and something like 'util' or 'etc_bindings' - I am in the process of rewriting of adding a compile-time abstraction layer to glsl and this will likely result in me rewriting how mgrl handles shader variables anyway. I need to think some more about this.

[wijnen]

I just force-pushed a commit that removes generated files from the patch, so you can expect some warnings when pulling; I hope this makes it easier to rebase if you change master. I'll work on moving world_matrix back into the shader object now.

[Aeva]

@wijnen I just merged a very large feature branch to master btw (glsl compiler); so I can help get this PR in shape for merging soon. I'm attempting to resolve the merge conflict right now.

[Aeva]

merge conflict is resolved, so you might want to verify that your changes still work

[wijnen]

THe test program I'm running (http://wijnen.dtdns.net/~shevek/2d-test/) isn't very extensive, but it still works fine.

[Aeva]

Hey - I refactored the branch in preparation for merging it. Some changes:

• please.dom.set_frame is now please.dom.set_context
• please.renderer an object defined in m.defs.js, which both set_context functions modify
• please.renderer.width and please.renderer.height are now getters that return what you'd expect
• removed the renderer.allow_picking thing since it is gl specific, and made it so the machinery is only added when using the gl renderer. In the future, I think it would not be hard to add picking functionality with a compatible api for the 2D renderer.
• I added a really pdq "nogl" demo so I can test things... given the output, I think its likely that I broke that functionality.

So, with the above in mind, could test this build and let me know if your stuff works? Also, I think there might be a performance regression in the webgl renderer that I need to investigate.

Overall, I'm pretty happy with this, and I think we can merge it soon. Sorry for delay in getting this together.

Also - I think I want to change how images are instanced in the DOM renderer, so that they are just a div with a background set, instead of an image inside of a div object. This will be faster to render, and it gives us a lot more control via css.

[wijnen]

I'm trying a more complex example (I didn't look at yours yet, perhaps it has the same issue). orthographic_grid seems to be ignored, and the full size of the overlay is always [-1:1] on both axes. I briefly tried to find the cause of this, but didn't find it. Perhaps some update code isn't run periodically (graph.sync is run; I checked).

I was afraid it might hurt the OpenGL performance. Since this is an extra feature, IMO that should be fixed before this can be merged.

[wijnen]

The placement problem seems to be in the overlay code, not in this pull request. I'll open a separate issue for it.