Page MenuHome

Viewport reconstruction was missing

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


Diff Detail

rB Blender
Build Status
Buildable 817
Build 817: arc lint + arc unit

Event Timeline

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.


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

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.


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.

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.

Marcelo Mutzbauer (1xundoredo) edited edge metadata.
  • 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.


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?)

This is now officially implemented in 2.8 via a different patch. Closing this one for now.