Fix T79714: Projecting texture from camera fails
ClosedPublic
Actions

Authored by Sybren A. Stüvel (sybren) on Aug 14 2020, 3:19 PM.

Details

Reviewers

Sergey Sharybin (sergey)
Pablo Dobarro (pablodp606)
Campbell Barton (campbellbarton)

Commits

rB5195d346f436: Fix T79714: Projecting texture from camera fails

Summary

Fix T79714: Wrong result when projecting texture from camera on object with modifier..

The evaluated mesh is now only recomputed when the required data layers are missing. Previously the evaluated mesh was re-evaluated incorrectly, which caused the stippled result and the failing assert.

Since now only the evaluated mesh is used, and never a temporary mesh, there is also no more need to keep track of whether the mesh needs freeing or not.

Diff Detail

Repository: rB Blender

Event Timeline

Sybren A. Stüvel (sybren) created this revision.Aug 14 2020, 3:19 PM

Harbormaster completed remote builds in B9524: Diff 27738.Aug 14 2020, 3:20 PM

Sybren A. Stüvel (sybren) requested review of this revision.Aug 14 2020, 3:20 PM

Sybren A. Stüvel (sybren) edited the summary of this revision. (Show Details)Aug 14 2020, 3:20 PM

Sybren A. Stüvel (sybren) edited the summary of this revision. (Show Details)

Sybren A. Stüvel (sybren) added projects: BF Blender, Sculpt, Paint & Texture.

Sybren A. Stüvel (sybren) edited the summary of this revision. (Show Details)

Sybren A. Stüvel (sybren) added a reviewer: Campbell Barton (campbellbarton).Aug 14 2020, 3:23 PM

I suggest we follow Git's style for wording in the commit message [1], namely:

Describe your changes in imperative mood, e.g. "make xyzzy do frotz" instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy to do frotz", as if you are giving orders to the codebase to change its behaviour.

Otherwise it sounds somewhat ambiguous. For example, "This does require reevaluating the geometry after the painting is done". Does it mean that the change causes re-evaluation? Or the eval_reset() requires to add re-evaluation somewhere?

[1] https://git.kernel.org/pub/scm/git/git.git/tree/Documentation/SubmittingPatches?id=HEAD

source/blender/editors/sculpt_paint/paint_image_proj.c
4032–4033	Does this work correctly if the mesh already had all the custom data layers in it? Maybe state explicitly in the comment to `BKE_object_eval_reset()` what happens in this case. And maybe look into avoiding mesh re-evaluation here if it has all the data already. Could be done by making it so `mesh_get_eval_final()` does re-set prior to re-evaluation (as re-evaluation without reset is meaningless anyway).
4034–4035	`create` always creates mesh, so you are the one who is responsible for freeing it.
4628–4629	How often `project_paint_end` is called? Tagging geometry for update in painting modes is something you should be doing with absolute care.

This revision now requires changes to proceed.Aug 17 2020, 9:24 AM

New approach after things were clarified in rBfc5eab357009

Harbormaster completed remote builds in B9609: Diff 27863.Aug 18 2020, 3:37 PM

Sybren A. Stüvel (sybren) edited the summary of this revision. (Show Details)Aug 18 2020, 3:39 PM

From reading looks good. Don't see anything obviously wrong.
No need for geometry tag either?

This revision is now accepted and ready to land.Aug 18 2020, 3:48 PM

In D8574#209916, @Sergey Sharybin (sergey) wrote:

No need for geometry tag either?

Nope, it all just keeps working like this. Because now the BKE_object_eval_reset(ob_eval); (which we had in an earlier version of this patch) is no longer necessary, no info is thrown away, so no need to recompute either.

Closed by commit rB5195d346f436: Fix T79714: Projecting texture from camera fails (authored by Sybren A. Stüvel (sybren)). · Explain WhyAug 18 2020, 3:58 PM

This revision was automatically updated to reflect the committed changes.

Sybren A. Stüvel (sybren) added a commit: rB5195d346f436: Fix T79714: Projecting texture from camera fails.