Page MenuHome

Fix T75539: Cycles. Update geometry when switching displacements
Needs RevisionPublic

Authored by Joan Bonet Orantos (LaTerreta) on Sep 15 2020, 11:58 AM.

Details

Summary

We only updated the mesh attribute request to undisplaced position
when switching from "Displacement and Bump" to "Bump Only", but
we forgot to do it when switching from "Displacement Only" to
"Bump Only"

Diff Detail

Repository
rB Blender
Branch
T75539 (branched from master)
Build Status
Buildable 10203
Build 10203: arc lint + arc unit

Event Timeline

Joan Bonet Orantos (LaTerreta) requested review of this revision.Sep 15 2020, 11:58 AM
Joan Bonet Orantos (LaTerreta) created this revision.
Brecht Van Lommel (brecht) requested changes to this revision.Sep 15 2020, 12:43 PM

Undisplaced positions are only needed for "Displacement and Bump", and always enabling them for "Displacement" wastes memory.

Adding them here will indirectly force the geometry to be updated, but we need a more direct solution. Probably the geometry could store if it has been displaced, and then if displacement method is bump and there is displaced geometry, it can be marked for update.

This revision now requires changes to proceed.Sep 15 2020, 12:43 PM
  • track if geometry was displaced, and mark it for update if displace method is BUMP

Did you mean something like this? I am getting familiar with the code, so I hope I am not that off

Hi @Brecht Van Lommel (brecht), as adviced on the blender-coders chat, I added "Render & Cycles" as reviewers too, given that I have seen in a recent meeting minutes that you asked for other's help on reviews.

Brecht Van Lommel (brecht) requested changes to this revision.Mon, Oct 12, 12:06 PM

It's individual geometry that should be marked as being displaced, not the geometry manager for all geometry.

I don't immediately have advice for how to best implement that, it may be a more complicated change.

This revision now requires changes to proceed.Mon, Oct 12, 12:06 PM

What about adding a bool member to Mesh class "is_displaced", and then set it to TRUE for any mesh that is displaced in GeometryManager::displace() (in mesh_displace.cpp). I may be wrong but it looks to me that by doing that at least we would cover the first part: having individual geometry (at least meshes) marked as being displaced. If that works, what if we then set

geometry_manager->need_update = true

if we find a Mesh in the scene with "is_displaced == true"? would that make sense? Perhaps is actually "Scene" class that can have a boolean too if any of the meshes in it was displaced and then mark it for an need_update as I did.

Yes, is_displaced should be a member of Geometry.

Probably the solution involves tagging shaders as having their displacement property changed, and then in the geometry update looping over the meshes and tagging them for updated based on the shader updates.