Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Control 3d crop region from 2d Viewer during IPR #2835

Closed
andrewkaufman opened this issue Oct 10, 2018 · 7 comments
Closed

Control 3d crop region from 2d Viewer during IPR #2835

andrewkaufman opened this issue Oct 10, 2018 · 7 comments
Assignees
@themissingcow
Labels
image Issues with GafferImage scene Issues with GafferScene ui Issues related to the UI/UX in Gaffer

Comments

@andrewkaufman
Copy link
Contributor

andrewkaufman commented Oct 10, 2018 •
edited by themissingcow
Loading

We've had this request at various points in time from Lookdev, Lighting, and FX at IE. Checking in recently, they're all still quite keen on it.

Feature Proposal

The Crop Window tool will be available in the 2D viewer as well as the 3D one. It will work in the same way (toolbar, left hand side, needs to be enabled). The window It will be editable on images that contain metadata pointing to a valid render node, with a suitable StandardOptions node upstream. The tool will reflect the current crop values in the scene at that part of the graph, rather than the crop window the image was rendered with, if the window has changed since rendering.

Gaffer will automatically populate render node metadata for new standard interactive renders.
@andrewkaufman andrewkaufman added image Issues with GafferImage scene Issues with GafferScene ui Issues related to the UI/UX in Gaffer labels Jun 20, 2019
@johnhaddon johnhaddon mentioned this issue Oct 18, 2019
Merged
@themissingcow themissingcow self-assigned this Jan 14, 2020
@themissingcow
Copy link
Contributor

@gregmassievfx @murrays-ie Are you able to run this by Lookdev and Lighting your end for me please?

@gregmassievfx
Copy link

That sounds great from our end, thanks.

@themissingcow
Copy link
Contributor

@gregmassievfx @murrays-ie - Are you able to ask around for any thoughts on some tweaks to the crop window tool UX?

Video: https://drive.google.com/open?id=1zqJO9O8vHMEvC9ESCrhbsiUZ0QDHD7El

Effectively:

  • The edges work as before, but with a larger hit area.
  • Dragging outside the crop region creates a new region.
  • Dragging inside the crop region moves the region around.
  • shift-dragging inside the crop region creates a new region.

The concession is that whilst the tool is active, with 3D views, you can no longer click-through the outside of the crop region (it never worked inside). You can still adjust the camera though using the usual modifiers anywhere in the view.

@murrays-ie
Copy link

Looks great Tom, nice additional UX touches! I'll be keen to try it in action.

I think the concession is okay, (I'd never actually noticed the existing click-through behaviour limitation with the crop tool active as I guess I tend to deactivate the crop tool once I'm done with it) I think people will likely trend to start using the crop tool predominately from the 2d viewer anyway, as that seems the more natural place for it.

@themissingcow
Copy link
Contributor

Looks great Tom, nice additional UX touches! I'll be keen to try it in action.

I think the concession is okay, (I'd never actually noticed the existing click-through behaviour limitation with the crop tool active as I guess I tend to deactivate the crop tool once I'm done with it) I think people will likely trend to start using the crop tool predominately from the 2d viewer anyway, as that seems the more natural place for it.

Cool thanks Murray. I just checked too, and the concession is even more obscure. I think it only applies when the camera move tool is also active. As I can't click through when its not anyway!

@gregmassie-ie
Copy link

gregmassie-ie commented Jan 24, 2020 via email

themissingcow pushed a commit to themissingcow/gaffer that referenced this issue Jan 24, 2020
@tomc-cinesite
RendererAlgo : Add render node name to output headers
672ef50 
In order to facilitate linking a render back to a point in the
graph, we pass the root-relative name of the render node as an
image header.

Improvements
------------

 - Rendering : Added the name of the render node to image metadata via
   the `gaffer:renderNode` header (GafferHQ#2835).

Breaking Changes
----------------

 - RendererAlgo : Changed signature for `outputOutput` to include
   the source scene plug (GafferHQ#2835).
themissingcow pushed a commit to themissingcow/gaffer that referenced this issue Jan 24, 2020
@tomc-cinesite
ImageView : Add CropWindowTool
8b869c9 
Features
--------

 - Viewer : Added the Crop Window Tool to image views to allow a scenes
   crop window to be adjusted directly from the rendered image (GafferHQ#2835).
@themissingcow themissingcow mentioned this issue Jan 24, 2020
Merged
themissingcow pushed a commit to themissingcow/gaffer that referenced this issue Jan 24, 2020
@tomc-cinesite
RendererAlgo : Add render node name to output headers
07a5bb9 
In order to facilitate linking a render back to a point in the
graph, we pass the root-relative name of the render node as an
image header.

Improvements
------------

 - Rendering : Added the name of the render node to image metadata via
   the `gaffer:renderNode` header (GafferHQ#2835).

Breaking Changes
----------------

 - RendererAlgo : Changed signature for `outputOutput` to include
   the source scene plug (GafferHQ#2835).
themissingcow pushed a commit to themissingcow/gaffer that referenced this issue Jan 24, 2020
@tomc-cinesite
ImageView : Add CropWindowTool
225c772 
Features
--------

 - Viewer : Added the Crop Window Tool to image views to allow a scenes
   crop window to be adjusted directly from the rendered image (GafferHQ#2835).
themissingcow pushed a commit to themissingcow/gaffer that referenced this issue Jan 31, 2020
@tomc-cinesite
ImageView : Add CropWindowTool
aa0f42a 
Features
--------

 - Viewer : Added the Crop Window Tool to image views to allow a scenes
   crop window to be adjusted directly from the rendered image (GafferHQ#2835).
themissingcow pushed a commit to themissingcow/gaffer that referenced this issue Jan 31, 2020
@tomc-cinesite
RendererAlgo : Add render node name to output headers
22e93e4 
In order to facilitate linking a render back to a point in the
graph, we pass the root-relative name of the render node as an
image header.

Improvements
------------

 - Rendering : Added the name of the render node to image metadata via
   the `gaffer:renderNode` header (GafferHQ#2835).

Breaking Changes
----------------

 - RendererAlgo : Changed signature for `outputOutput` to include
   the source scene plug (GafferHQ#2835).
themissingcow pushed a commit to themissingcow/gaffer that referenced this issue Jan 31, 2020
@tomc-cinesite
ImageView : Add CropWindowTool
86a1b1f 
Features
--------

 - Viewer : Added the Crop Window Tool to image views to allow a scenes
   crop window to be adjusted directly from the rendered image (GafferHQ#2835).
themissingcow pushed a commit to themissingcow/gaffer that referenced this issue Jan 31, 2020
@tomc-cinesite
RendererAlgo : Add render node name to output headers
dee6ef0 
In order to facilitate linking a render back to a point in the
graph, we pass the root-relative name of the render node as an
image header.

Improvements
------------

 - Rendering : Added the name of the render node to image metadata via
   the `gaffer:renderNode` header (GafferHQ#2835).

Breaking Changes
----------------

 - RendererAlgo : Changed signature for `outputOutput` to include
   the source scene plug (GafferHQ#2835).
themissingcow pushed a commit to themissingcow/gaffer that referenced this issue Jan 31, 2020
@tomc-cinesite
ImageView : Add CropWindowTool
0ddde37 
Features
--------

 - Viewer : Added the Crop Window Tool to image views to allow a scenes
   crop window to be adjusted directly from the rendered image (GafferHQ#2835).
themissingcow pushed a commit to themissingcow/gaffer that referenced this issue Feb 3, 2020
@tomc-cinesite
RendererAlgo : Add render node name to output headers
69ccccf 
In order to facilitate linking a render back to a point in the
graph, we pass the root-relative name of the render node as an
image header.

Improvements
------------

 - Rendering : Added the name of the render node to image metadata via
   the `gaffer:renderNode` header (GafferHQ#2835).

Breaking Changes
----------------

 - RendererAlgo : Changed signature for `outputOutput` to include
   the source scene plug (GafferHQ#2835).
themissingcow pushed a commit to themissingcow/gaffer that referenced this issue Feb 3, 2020
@tomc-cinesite
ImageView : Add CropWindowTool and improve message presentation
871301b 
A note on presentation changes:

Using the overlay box is more consistent with other tools and works
better when the view can be panned/zoomed as per ImageView.

It caused a problem with the ImageView though, as the color inspector
is center-aligned, but the crop overlay is left. As we don't want to
have to adjust the CropWindowToolUI presentation based on which view
is hosting, we compromise by making the color inspector wider, so it
aligns on the left side, but keeping the color readouts centred so they
are closer to the view point.

Ideally we'd have view safe areas so we don't need to do this all with
hard-coded margins, etc.

Had some issues using the spacer-widget trick employed by other tools, a
more robust solution seemed to be to instead make use of stylesheet
margins to create the appropriate inset.  We can't use these right now
for Transform tool as there are other plug value widgets in there that
aren't uniquely accessible via a suitable selector.

Features
--------

 - Viewer :
   - Added the Crop Window Tool to image views to allow a scenes crop
     window to be adjusted directly from the rendered image (GafferHQ#2835).
   - Changed Crop Window Tool status presentation to match other
     tools (GafferHQ#2835).
themissingcow pushed a commit to themissingcow/gaffer that referenced this issue Feb 3, 2020
@tomc-cinesite
ImageView : Add CropWindowTool and improve message presentation
decc9e6 
A note on presentation changes:

Using the overlay box is more consistent with other tools and works
better when the view can be panned/zoomed as per ImageView.

It caused a problem with the ImageView though, as the color inspector
is center-aligned, but the crop overlay is left. As we don't want to
have to adjust the CropWindowToolUI presentation based on which view
is hosting, we compromise by making the color inspector wider, so it
aligns on the left side, but keeping the color readouts centred so they
are closer to the view point.

Ideally we'd have view safe areas so we don't need to do this all with
hard-coded margins, etc.

Had some issues using the spacer-widget trick employed by other tools, a
more robust solution seemed to be to instead make use of stylesheet
margins to create the appropriate inset.  We can't use these right now
for Transform tool as there are other plug value widgets in there that
aren't uniquely accessible via a suitable selector.

Features
--------

 - Viewer :
   - Added the Crop Window Tool to image views to allow a scenes crop
     window to be adjusted directly from the rendered image (GafferHQ#2835).
   - Changed Crop Window Tool status presentation to match other
     tools (GafferHQ#2835).
@themissingcow
Copy link
Contributor

Implemented in #3582 in 0.56.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@themissingcow themissingcow

Labels
image Issues with GafferImage scene Issues with GafferScene ui Issues related to the UI/UX in Gaffer
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@themissingcow @andrewkaufman @gregmassie-ie @murrays-ie @gregmassievfx