Viewport reconstruction was missing
Needs ReviewPublic

Authored by Marcelo Mutzbauer (1xundoredo) on Sep 10 2017, 8:52 PM.

Details

Diff Detail

Repository
rB Blender
Branch
_viewport_reconst
Build Status
Buildable 817
Build 817: arc lint + arc unit
Sergey Sharybin (sergey) requested changes to this revision.Sep 11 2017, 9:13 AM

Not fan of adding runtime fields to such rather low level DNA structures. Would imagine having VBO in camera itself will localize everything related on drawing of camera.

source/blender/draw/modes/object_mode.c
1391

This doesn't seem to be aware of custom track color?

source/blender/makesdna/DNA_tracking_types.h
170 ↗(On Diff #9252)

This matrix is dependent on viewport setting, which means your logic will fail when you'll try showing reconstruction in two viewports. Also, same movie clip can be used by different cameras in different scenes and those cameras might have different scale.

This revision now requires changes to proceed.Sep 11 2017, 9:13 AM
Clément Foucault (fclem) requested changes to this revision.Sep 11 2017, 1:43 PM

Appart from the missing custom color I'm ok with the patch as it is. The DNA overload is bad design from me. I may have plans to change it.

source/blender/draw/modes/object_mode.c
1391

To the author, there is a GPU_SHADER_INSTANCE_VARIYING_COLOR_VARIYING_SIZE if the color can change per instance. or you could create a shading group in this function and pass the color as a uniform.

source/blender/makesdna/DNA_tracking_types.h
170 ↗(On Diff #9252)

This will works for now because it's always updated before drawing. But I agree the current design of putting attrib/uniform in DNA is flawed and should be reconsidered.

  • Attempt to get rid of runtime DNA values (only used for values that used to be added by this patch, will post comment with clarification on how it works)
  • minor cosmetic changes

Clarification for how attributes are now stored:

There is now a DRW_shgroup_call_dynamic_add_copy, which creates a draw call, however it won't store pointers to the attrib data, but a copies of the data itself.
Current code does this as an option, the code might get simpler if we decided to use this method exclusively.
This could easily be reverted in case this is not the way to go.

This should probably be considered in a separate patch, however having it here might help to illustrate how it would be used.

source/blender/draw/modes/object_mode.c
1391

Hmm... The color attribute is getting passed to the shaders for the empty shading groups. These shaders are of GPU_SHADER_INSTANCE_VARYING_COLOR_VARYING_SIZE.
Testing also confirms everything works as expected, even when tweaking theme colors.
(I assume by custom track color you're referring to theming?)