Page MenuHome
Differential D12750

Compositor: Support backdrop offset for the Viewer node
Changes PlannedPublic
Actions

Authored by Jeroen Bakker (jbakker) on Oct 4 2021, 9:23 PM.

Details

Reviewers
None
Commits
rB33409f9f1cd4: Compositor: Support backdrop offset for the Viewer node
Summary

This is only part of the experimental "Full Frame" mode (disabled
by default). See T88150.

Currently the viewer node uses buffer paddings to display image offset
in the backdrop as a temporal solution implemented for D12466: Compositor: Add support for canvas compositing.
This solution is inefficient memory and performance-wise. Another
issue is that the paddings are part the image when saved.

This patch instead sets the offset in the Viewer node image
as variables and makes the backdrop take it into account
when drawing the image or any related gizmo.

  • Update to latest image engine.
  • Fix issues that were detected when it was part of master. It might be that this is a no-op as image engine did had some short-comings that has been fixed last month. In that case we should just test.

Diff Detail

Repository
rB Blender

Event Timeline

Manuel Castilla (manzanilla) requested review of this revision.Oct 4 2021, 9:23 PM
Manuel Castilla (manzanilla) created this revision.
Harbormaster completed remote builds in B17554: Diff 42902.Oct 4 2021, 9:23 PM
Manuel Castilla (manzanilla) edited the summary of this revision. (Show Details)Oct 4 2021, 9:25 PM
Manuel Castilla (manzanilla) added a reviewer: Jeroen Bakker (jbakker).
Manuel Castilla (manzanilla) added a project: Compositing.
Manuel Castilla (manzanilla) added a comment.Oct 4 2021, 9:43 PM

@Jeroen Bakker (jbakker) I've tried to implement this setting the offset in the viewer node but a lot image/gizmo update issues come up, mainly due to the thread node tree has to merged back to the original tree, so setting the offset has some delay.

In this solution still gizmos doesn't update some times, but it's more consistent and the code more simple. Will look how to fix the gizmo updates.

This is the diff for the unfinished node solution (don't support gizmo updates and node groups):

alt_backdrop.diff19 KBDownload

Manuel Castilla (manzanilla) updated this revision to Diff 42948.Oct 6 2021, 12:01 AM
  • Ensure gizmos are updated after compositing (fixes the issue)
  • Remove unneeded whitespace
  • Take into account viewer offset in Crop, Sun Beams and Corner Pin gizmos
Harbormaster completed remote builds in B17584: Diff 42948.Oct 6 2021, 12:01 AM
Manuel Castilla (manzanilla) updated this revision to Diff 43325.Oct 14 2021, 9:04 PM
  • Merge branch 'master' into cmp-viewer-offset2
Harbormaster completed remote builds in B17840: Diff 43325.Oct 14 2021, 9:04 PM
Manuel Castilla (manzanilla) updated this revision to Diff 43328.Oct 14 2021, 9:19 PM
  • Cleanup: fix comment
Harbormaster completed remote builds in B17842: Diff 43328.Oct 14 2021, 9:19 PM
Jeroen Bakker (jbakker) added a comment.Oct 27 2021, 8:31 AM

Need to test drive this if all backdrop operations (fit) work as expected.

source/blender/makesdna/DNA_image_types.h
203

Should mention in what space so it is clear where the origin is and how to work with the values .

  • pixel space: 1 unit is a single pixel on screen.
  • texel space: 1 unit is a single texel of image.
  • canvas space: 1 unit is a single texel of image.
Manuel Castilla (manzanilla) updated this revision to Diff 44092.Oct 29 2021, 1:55 AM
  • Merge branch 'master' into cmp-viewer-offset2
  • Specify viewer image offsets are in pixel space
Harbormaster completed remote builds in B18302: Diff 44092.Oct 29 2021, 1:56 AM
Manuel Castilla (manzanilla) marked an inline comment as done.Oct 29 2021, 1:57 AM
Jeroen Bakker (jbakker) accepted this revision.Nov 5 2021, 2:01 PM
Jeroen Bakker (jbakker) added inline comments.
source/blender/draw/engines/image/image_engine.c
68 ↗(On Diff #44092)

I update the image engine to make it more extensible, but therefore this change needs to be done in image_space_node.hh#SpaceNodeAccessor.get_image_mat

This revision is now accepted and ready to land.Nov 5 2021, 2:01 PM
Aditia A. Pratama (aditiapratama) awarded a token.Nov 20 2021, 1:01 AM
Jeroen Bakker (jbakker) requested changes to this revision.Jan 4 2022, 8:55 AM

Doesn't seem to work anymore and needs some changes to proceed.

This revision now requires changes to proceed.Jan 4 2022, 8:55 AM
Habib Gahbiche (zazizizou) updated this revision to Diff 48305.Feb 13 2022, 6:37 PM
Habib Gahbiche (zazizizou) added a subscriber: Habib Gahbiche (zazizizou).
  • rebased on latest master
  • moved image matrix computation to SpaceNodeAccessor

Transformed images e.g. using Transform now show correctly as a backdrop or in image editor. However, results are not updated properly. Probably due to T95699

Harbormaster completed remote builds in B20535: Diff 48305.Feb 13 2022, 6:37 PM
Jeroen Bakker (jbakker) requested changes to this revision.Feb 14 2022, 9:40 AM
Jeroen Bakker (jbakker) added inline comments.
source/blender/draw/engines/image/image_drawing_mode.hh
164

Although this works, it breaks the idea of being screen spaced textures and can lead to other artifacts when we want to optimize panning in the image editor/node editor.

The idea is to update the clipping_uv_bounds of the texture info so that the viewer result is copied into the right part of the screen space texture. This could be done by altering the init_ss_to_texture_matrix so that update_screen_uv_bounds would set the correct offset.

This revision now requires changes to proceed.Feb 14 2022, 9:40 AM
Habib Gahbiche (zazizizou) updated this revision to Diff 48742.Feb 27 2022, 5:25 PM

Moved offset computation to AbstractSpaceAccessor::init_ss_to_texture_matrix() as requested by reviewer

Harbormaster completed remote builds in B20746: Diff 48742.Feb 27 2022, 5:25 PM
Habib Gahbiche (zazizizou) mentioned this in T88150: Full Frame Compositor.Mar 1 2022, 9:01 PM
Jeroen Bakker (jbakker) accepted this revision.Mar 15 2022, 2:17 PM

Seems to work as expected! Thanks for your patience

This revision is now accepted and ready to land.Mar 15 2022, 2:17 PM
Closed by commit rB33409f9f1cd4: Compositor: Support backdrop offset for the Viewer node (authored by Habib Gahbiche (zazizizou)). · Explain WhyMar 16 2022, 11:53 PM
This revision was automatically updated to reflect the committed changes.
Habib Gahbiche (zazizizou) added a commit: rB33409f9f1cd4: Compositor: Support backdrop offset for the Viewer node.
Habib Gahbiche (zazizizou) added a comment.Mar 16 2022, 11:54 PM

Thanks for reviewing @Jeroen Bakker (jbakker) !

Brecht Van Lommel (brecht) reopened this revision.Mar 19 2022, 12:42 AM
Brecht Van Lommel (brecht) added a subscriber: Brecht Van Lommel (brecht).

Reverted in rB7fed213ba40d: Revert "Compositor: Support backdrop offset for the Viewer node" since this causes a critical bug and no fix was committed in the last two days.

This revision is now accepted and ready to land.Mar 19 2022, 12:42 AM
Brecht Van Lommel (brecht) added a reverting change: rB7fed213ba40d: Revert "Compositor: Support backdrop offset for the Viewer node".Mar 19 2022, 12:42 AM
Habib Gahbiche (zazizizou) mentioned this in D14388: Compositor: Support backdrop offset for the Viewer node.Mar 19 2022, 4:54 PM
Ethan Hall (Ethan1080) added a subscriber: Ethan Hall (Ethan1080).Mar 19 2022, 7:54 PM
Ethan Hall (Ethan1080) added inline comments.
source/blender/draw/engines/image/image_space_image.hh
156

Inconsistent formatting: sima ->zoom sima->zoom

167

The addition of offset_x and offset_y causes issues for the image editor. See: T96543

173

The block of code above was accidently indented.

Habib Gahbiche (zazizizou) added a comment.Mar 19 2022, 8:03 PM

@Ethan Hall (Ethan1080) thanks for the comments. I addressed the issues in a new revision: D14388. Feel free to review that one as well.

Jeroen Bakker (jbakker) commandeered this revision.Jan 11 2023, 9:26 AM
Jeroen Bakker (jbakker) edited reviewers, added: Manuel Castilla (manzanilla); removed: Jeroen Bakker (jbakker).
This revision now requires review to proceed.Jan 11 2023, 9:26 AM
Jeroen Bakker (jbakker) planned changes to this revision.Jan 11 2023, 9:27 AM
Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)
Jeroen Bakker (jbakker) removed a reviewer: Manuel Castilla (manzanilla).

Revision Contents
Changeset List

PathSize
source/
blender/
compositor/
operations/
3 lines
46 lines
draw/
engines/
image/
1 line
7 lines
2 lines
1 line
33 lines
9 lines
editors/
space_node/
6 lines
43 lines
7 lines
makesdna/
3 lines

Diff 49359

View Options

source/blender/compositor/operations/COM_ViewerOperation.h

Loading...
View Options

source/blender/compositor/operations/COM_ViewerOperation.cc

Loading...
View Options

source/blender/draw/engines/image/image_drawing_mode.hh

Loading...
View Options

source/blender/draw/engines/image/image_engine.cc

Loading...
View Options

source/blender/draw/engines/image/image_instance_data.hh

Loading...
View Options

source/blender/draw/engines/image/image_space.hh

Loading...
View Options

source/blender/draw/engines/image/image_space_image.hh

Loading...
View Options

source/blender/draw/engines/image/image_space_node.hh

Loading...
View Options

source/blender/editors/space_node/drawnode.cc

Loading...
View Options

source/blender/editors/space_node/node_gizmo.cc

Loading...
View Options

source/blender/editors/space_node/space_node.cc

Loading...
View Options

source/blender/makesdna/DNA_image_types.h

Loading...