Refactoring Object Picking System

[Aeva]

This issue is part of #232, and also relates to #192.

This encompasses the following tasks:

• [x] Coarsely decouple the picking functionality from GraphNode and SceneGraph, and remove any specific picking functionality from m.graph.js and other files and move them to m.picking.js. The purpose of doing this is to make the code easier to understand and maintain.
• [x] Move the enabling / settings mechanism from the SceneGraph into the please.picking namespace.
• [x] Rework picking pass functionality to use the RenderNode singleton. Ensure the new system works sans actual picking.
• [x] rename "decode_color" to "select_for_picking_color" or something like that? also move "picking_pass" into a less public looking namespace.
• [x] Fix the draw function on the RenderNode singleton such that this system actually can work.
• [x] Bring system to working state.
• [x] Update picking pass to use indirect rendering correctly. Verify whole system works.
• [x] Update API such that "pick_location" can be set either per object or globally(?) See #192
• [x] Update event dispatcher stuff to use signals. Try to factor out as much dispatching code from m.graph.js as possible.
• [x] New mechanism for attaching a compositor to the picking graph. See comments below.
• [x] Updated picking pass to only render to a small tile (eg 1x1), remove special case from stream callback stuff.
• [x] Correct the X (and Y?) offset of the pick coordinates. Also, pick world coordinates seem to be clamping - this might mean the scale is wrong in the shader...?? You can see this in the location_picking demo.
• [x] Rate limiting picking for move events? note Produces desired results at low sampling rates, but predictably also produces artifacts D:
• [x] some notion of "layers" / mode changing (needed to implement any sort of click drag well)
• [x] Update pick id stuff to not use either flat or statics, but some other lookup, such that everything can have indexes generated. EDIT: __all_drawables is that lookup now, which at the time of writting is just statics and flat concatenated together.
• [x] Verify cache invalidation hack doesn't break anything not obvious.
• [x] Ensure that all demos which use picking have been ported to the new API.
• [x] Update docstrings in m.graph.js
• [x] Add docstrings to m.picking.js

[Aeva]

It occurs to me that there is another benefit to using indirect rendering for picking - we don't need to re-render the scene for every mouse event.

Instead, rendering the picking buffers would only happen upon a mouse event if the current buffers are out of date. We could have the picking node be frame limited at 15 or 30hz, and only render if some dirty flag is set.

The dirty flag would be set by the mouse event handlers, and is unset by the picking node when the event queue is empty.

Mouse events then could be processed out of phase with the render loop, however a mouse event triggers the dirty flag, then processing the events should be blocked until after the buffers are redrawn.

The sampling frequency should be exposed as a picking option.

[Aeva]

Picking RenderNode is generating bad source code:

// BEGIN GENERATED GRAPH RENDERING CODE

this.prog.vars[mgrl_model_local_min] = [object Float32Array];
this.prog.vars[mgrl_model_local_size] = 4,4,0.48876953125;
this.prog.attrs['position'].enabled = true;
this.prog.vars['mgrl_model_local_min'] = [object Float32Array];

Note that it is generating arrays wrong in two different ways.

[Aeva]

an even better idea than running the picking passes at a lower frame rate and lower resolution is to be more like a tiling renderer, and only render the pixel under the mouse

[Aeva]

Rearranged some of the tasks above. I removed the MRT one since I'm not sure there is as much benefit to it at the moment. I think the tiling thing will be a much bigger improvement, such that it might not be necessary to also add in MRT stuff.

The tiling stuff might appropriately go into its own ticket later too, though it should not be hard to implement.

[Aeva]

Up next is to do the tiling optimization and throw together a new way to do composited picking coordinates.

Composited picking unfortunately doesn't play nicely if I'm only rendering the area directly under the mouse for picking. Two work-arounds come to mind so far, ordered in preference:

• Drop this feature entirely, and instead replace it with the ability to specify a distortion function which is called on the mouse coords and generates new coords. Easy to implement, much faster, but can't simply reuse the shader from the compositing pass.
• Provide some kind of coordinate texture to run through the compositing pass instead. Render the compositing pass like normal, but use the picked value as the new mouse coords, and then finish the remainder of the picking process as normal. More complicated to implement, adds more context switches than I'd like, but allows code reuse on the user's side in the "omg magic" kind of way.

I think I'm going to go with the picking function, since this is kind of a niche feature.

UPDATE I went with the picking function, and it works better than the original implementation :D

[Aeva]

Grep tells us that the following demos need to be ported:

• [x] demos/distorted_selection/project.js
• [x] demos/object_selection/project.js
• [x] demos/location_picking/project.js
• [x] demos/dynamic_glsl/project.js
• [ ] demos/bezier_pick/demo.js
• [ ] demos/slow_bezier/demo.js

[Aeva]

bezier_pick demo is converted to the new API, but the demo is now broken.

[Aeva]

Picking pass now only renders into a small tile! 8x8 seems to be the minimum it wants to do, it works fine with that.

The pick coordinates seem to be slightly wrong though, but that should be easy enough to fix.

Performance-wise seems to be marginally better? Enabling move events in the location_picking demo seems to yield 20-30 fps rather than 15ish(??). Last figure is from memory, but I can't imagine this hurting anything at least.

I guess the other thing to try is probably going to be to frame limit when the picking pass can run (not the rendenode but the outter logic itself, because otherwise we'll just get cached results from previous passes). Or just rate limit move events and accept all clicky events.

[Aeva]

Updated one of the bezier demos to use the layers mechanism, but it doesn't quite work yet. The reason for this is probably because the picking IDs are only being set for static objects. The demo uses "floor node", which is only able to run in the dynamic path at the moment.

What I should do here is first update the picking system so that the color ids are generated correctly for all objects etc, and then probably re-work the floor node so that it can generate IR and run static.

[Aeva]

There were some mystery changes on my computer which I've checked in to their own branch ("loose_end") and pushed up here. You can see them here 12c1e8d8bafc1d6. At the time of writing, I don't remember what I was doing with them, ~but it broke the build.~

Correction: The index demo is broken, but that is consistent with the fast_graph branch. The distorted picking demo works fine.

[Aeva]

Digging into the "loose_end" branch a bit, it looks like I was trying to do the following:

1. updated bezier_pick demo to use a "layer" based approach to allow multiple picking graphs
2. added some sort of "__all_drawables" array, which was probably intended to remedy the problem of only being able pick static objects

I'm pretty sure the layers functionality already exists in pick_nouveau and I probably just forgot to check in the changes to the demo before attempting to monkey around with other functionality.

So my guess is, the fact that nothing happens when you click on stuff in the bezier_pick demo is likely a problem with the __all_drawables thing, and not the other stuff.

[Aeva]

Further poking around confirms:

1. can click to select the control points
2. picking layer then changes to the "drag" layer
3. picking layer reverts to "select mode" on mouse up

So, most of the machinery here is working right, and the problem is probably in the rendering function for 'drag mode'.

[Aeva]

Commit bcbca69b665a363e9460a03071d3a95d71b769d3 adds the sourceURL directive to generated code. This should help a lot in debugging the loose_end branch.

[Aeva]

It looks like the secondary picking shader should be getting the right object_id assigned now for the floor, and the draw calls are being hit correctly for that. Sticking a break point in please.picking.__etc.picking_pass though says its only reading [0,0,0,0] from the picking pass :/ so something isn't working right still.

[Aeva]

I'm running out of ideas, so I think the best course of action is to solve an adjacent problem and hope it fixes this one too:

Currently there is just one rendernode "singleton" for the picking system. This is a problem, because it requires hotswapping the GraphNode for it, triggering a recompile event for the picking pass. A better solution would be to just generate a RenderNode for each tracked graph node.

Commit 30bca546eef5322071bfe6c115f02282d2b888c2 adds this functionality, but it does not solve my problem D:

[Aeva]

As best I can tell, the following is true now:

• there are now separate render nodes per picking graph.
• you can easily break on either picking pass's draw functions
• both picking passes are called when expected
• the object_index property is populated when the draw functions are compiled
• the draw call for the object in the second picking pass gets called
• the object_index variable actually gets uploaded
• picking pass for the graph that has the control points in it works fine all of the static objects will render fine in the picking passes, but the FloorNode objects wont.

The space expected to be occupied by the FloorNode objects in the picking graph is always [0, 0, 0, 0] somehow.

edit AAAAAH ok found a pattern here - there is a FloorNode instance in both graphs, and in both graphs the color picked is black.

Other things to note:

• tried disabling culling, though that didn't seem to make a difference
• correctly picked objects' color values have 255 in their alpha channel, not 0, so it's selecting the clear color for some reason :P

[wijnen]

The idea of picking is that you render the scene with a special shader offscreen, and then read a pixel from the render result, right? I'm probably missing something, but it seems that the easiest way to debug this is to draw the result of the picking render on screen?

[Aeva]

Basically, yeah. I'm adapting the old object selection system over to use the fancy new rendering infrastructure that I've been working on for the last half year.

So with regards to drawing out the buffer somewhere - I could do that, but I can get the same information that would give me by reading out the color picked, which is relatively easy to do. But by doing essentially that, I know that it is working for some objects, but not others in this case.

So in particular, consider this generated picking pass: https://gist.github.com/Aeva/8bc47097c4d7c7e3381c5d6549023953

Everything from lines 29 through 221 renders correctly.

Then on lines 230 and 231, a single "legacy renderable" (the floor node) should be drawn. The bind and draw methods DO get called, and the old drawing infrastructure for it actually does execute, but that area of the buffer ends up still being just the clear color.

[Aeva]

Ok, I fixed that blocker finally, though I'm not sure why it works now. I removed the tiling optimization for picking, so it renders to a normal buffer now, and as a result, the FloorNode in the bezier demo is now selectable.

I also updated the FloorNode to use the new rendering path, really only for ease of debugging, but everything should use that eventually.

I've not yet looked into why yet, but the location picking results for the floor node are very wrong - I suspect it relates to the location picking stuff being off for the location_picking demo, too. Object selection seems to be on point, however.

Also: I merged loose_end into pick_nouveau and deleted loose_end.

[Aeva]

The value of loc_color on this line is always [1, 0, 0, 255] in the bezier curve demo as is, when you try to drag around a marker regardless of which marker. Otherwise, it seems the demo is working as expected now.

In the location_picking demo the value of loc_color is constant for a given object, but different for different objects. It is somewhat consistent spatially, so the demo is somewhat more functional. If the demo were to use a FloorNode for location picking, it would likely behave similarly to the bezier curve demo.

For the bezier_curve demo, the output of gl.readPixels for the entire buffer reveals it to be a consistent color. Also, the inputs look fine (vbo.stats.min and vbo.stats.size appear to be specified correctly) and it looks to be in the correct mode renderer.shader.mgrl_select_mode = false.

From the behavior as is, it looks like it could be either that the local_position varying var is wrong, or mgrl_select_mode is actually true somehow, and its just a coincidence that the location picking demo seems approximately correct.

• local_position seems fine (position * mgrl_orthographic_scale)
• loc_color == id_color so that means its still in select mode somehow :/

Hah, found it! The problem is caused by the caching mechanism in the compositor. The picking pass renders once for selection, writes to the buffer and caches it, and then the picking pass is rendered a second time, but the cached result is returned, thus the values are wrong for location picking. Oops!

[Aeva]

This issue is basically complete now, to recap from the checklist at the top of the page, the following needs to be done before this can be closed:

• [x] Verify cache invalidation hack doesn't break anything not obvious.
  • __cached_framebuffer doesn't trigger any registration code when null; its merely overwritten every frame to indicate that it was ever written to
  • picking.js implements its own rate limiting, so we're good here
• [x] Ensure that all demos which use picking have been ported to the new API.
• [x] Update docstrings in m.graph.js
• [x] Add docstrings to m.picking.js

[Aeva]

The slow_bezier demo appears to have a regression (move a control point and you'll see it) - there will be lamps drawn at the origin that never move.

What is actually happening, is that the demo would normally set their visibility to false. The demo is unable to do this because the new rendering pathway doesn't support changing visibility yet. So, this is not a bug in the demo, but a missing feature in the new rendering pipeline.