search

GafferHQ / gaffer

Gaffer is a node-based application for lookdev, lighting and automation
http://www.gafferhq.org
BSD 3-Clause "New" or "Revised" License
968 stars 207 forks source link

Design : Catalogue Improvements #3646

Open themissingcow opened 4 years ago

themissingcow commented 4 years ago

User Story

What

As a User I'd like to be able to know more about the images in a Catalogue node.

Why

Presently, it isn't possible to determine whether an image is a currently running/completed render, or an imported image from disk. I'd also like to know more about specifics of each image, such as frame number, render layer, or render global settings. This would allow me to make more objective or informed decisions about relative image differences.

Proposal

Catalog Phase 1 v3

Note: topics such as in-viewer overlays, etc. are to be covered under a separate topic. Note: Icons aren't final and can be refined/adjusted during implementation.

Technical Notes

Render log integration requires #3419.

Artwork.zip

themissingcow commented 4 years ago

@andrewkaufman @gregmassie-ie @murrays-ie

andrewkaufman commented 4 years ago

Some first glace thoughts:

Beyond that, it'd be nice to refer to some future phase2 viewer overlay idea in the description, just so people know its been left out of this design intentionally.

danieldresser-ie commented 4 years ago

The UI looks fairly nice. My current issues with the catalog are more pedestrian: sometimes IPR images randomly pick a slot in the middle of the catalog instead of at the end, and when you snapshot an IPR image ( using the duplicate button, which could probably be named Snapshot ), the snapshot appears below the live IPR render, instead of above it, so that things end up out of temporal order.

themissingcow commented 4 years ago

My current issues with the catalog are more pedestrian: sometimes IPR images randomly pick a slot in the middle of the catalog instead of at the end, ... when you snapshot an IPR image ( using the duplicate button, which could probably be named Snapshot ), the snapshot appears below the live IPR render, instead of above it, so that things end up out of temporal order.

Which version is this with? We did some work a while back to try and improve this (https://github.com/GafferHQ/gaffer/pull/3570 55.4.1, https://github.com/GafferHQ/gaffer/pull/3555 55.3.0), they should duplicate above the still rendering image.

themissingcow commented 4 years ago

  • Type Column

    • Is a completed/stopped IPR render now an "on-disk" type?
    • Is a non-progressive remote render and "in-progress" type? If so, do we want to indicate the non-iprness of it?

The Catalog saves images received from a display driver to disk when it closes the connection. The current idea was simply that the distinction was 'in-progress, ie: a buckets coming in from a display driver' .vs. 'saved on disk'. In the future, when we do catalog slot reservations as part of a render launch, there then may be a 'pending' type too.

Any thoughts on a consensus of people your end if they would specifically want a distinction between what kind of render, rather than just 'its still rendering'?

  • Columns

    • Do you have any thoughts on out-of-box columns? The 3 you've shown in the drawing?

The drawing was just meant to be notional - haven't yet thought about exactly which out the box columns we'd provide - any suggestions?

  • Is the "configuring" done only via python/startup configs, or will users have a UI for adding columns?

Was thinking via config rather than a UI at this point, if that sounds OK?

  • Extended properties

    • Is the Meta tab a pseudo ImageInspector? Will it be able to diff metadata between catalogue images?

    • Should we finally make an ImageInspector instead (perhaps still embedding it in that tab as well)?

Depends how much effort we want to put in at this point, It would certainly be done as an embedded ImageInspector, I guess it's just how featured we want to make this time around.

Beyond that, it'd be nice to refer to some future phase2 viewer overlay idea in the description, just so people know its been left out of this design intentionally.

Yeah, I'm on with that this week. The Phase 1 here was specifically about Catalogue node changes.

andrewkaufman commented 4 years ago

Any thoughts on a consensus of people your end if they would specifically want a distinction between what kind of render, rather than just 'its still rendering'?

Yes, they want to distinguish the kind of render. One person mentioned it doesn't necessarily need to be via the Type column, could be a custom column here instead (eg "Detailed Type"). But if it could be done via the main Type column, I'd be very happy.

Was thinking via config rather than a UI at this point, if that sounds OK?

It feels a bit unfortunate tbh... It means the column configuring is limited to script-saavy folks, and also means you can't easily configure per-catalogue. I wouldn't imagine the UI itself is too hard (just a context menu on that ... mini-column), and the serialization is just metadata for which columns are shown for this node, so I guess the hard part here is deciding what to put in the context menu? I wonder if just listing the header metadata from the existing catalogue entries might be enough? Maybe it'd need to be coupled with parsing the renderer stats file for keys as well, which gets a bit fiddly, but still hopefully isn't too difficult...

It would certainly be done as an embedded ImageInspector, I guess it's just how featured we want to make this time around.

If it's going to be done that way regardless, I'd vote to put the extra effort in and make a nice ImageInspector. We've been wanting one for ages, and releasing one without the same UX as the globals tab of the SceneInspector would feel odd... search filtering, diffing, drag/dropping values are all very valuable debugging tools.

gregmassie-ie commented 4 years ago

Great stuff Tom!

Just regarding the stages of a preview render, I would ideally want to know if it is:

additionally, I would like to know if it's a local render or farm render.

A column with "disk location" would be handy too, although I would imagine it would be usually hidden unless required.

johnhaddon commented 4 years ago

I'd vote to put the extra effort in and make a nice ImageInspector. We've been wanting one for ages, and releasing one without the same UX as the globals tab of the SceneInspector would feel odd... search filtering, diffing, drag/dropping values are all very valuable debugging tools.

I've been wanting an ImageInspector for a long time, and planned to do it by generalising the SceneInspector components. But now's rather an awkward moment for that, as the advent of EditScopes is about to raise lots of questions about how exactly the SceneInspector should be re-implemented. I'm reluctant to do a generalisation without taking the big picture into account, and I think that puts it out of scope for Phase 1.

andrewkaufman commented 4 years ago

Fair enough. So back to my original question, is the Meta tab a pseudo ImageInspector? What features will it have for phase 1? Or is the Meta tab removed from phase 1?

themissingcow commented 4 years ago

Fair enough. So back to my original question, is the Meta tab a pseudo ImageInspector? What features will it have for phase 1? Or is the Meta tab removed from phase 1?

The idea for phase one was that it would probably be a very basic pseudo-inspector, so you can at least see what header fields are there and their values without having to print them in the python editor, to aid with column configuration and general inspection.

We can totally make the columns configurable per-catalog if its genuinely useful, this was just a straw-person to work out how much effort we want to prioritise into this at the moment in relation to the IPR async/render control work, and the other things on the table. (btw, the ... column isn't meant to imply there are only three or a special one, more just a visual "et. cetera').

Phase 1 was really trying to get the low hanging fruit, and it can flesh out over time, unless it's significantly less useful without.

Either way, I'll update the design with a refined status nee type column that better describes how we might make it work with farm/local/etc...

themissingcow commented 4 years ago

Just regarding the stages of a preview render, I would ideally want to know if it is: ... additionally, I would like to know if it's a local render or farm render.

Thanks Greg, will revise the design with a more featured column for this.

A column with "disk location" would be handy too, although I would imagine it would be usually hidden unless required.

Thats cool, I think path would def be one of the catalogue-standard columns you could add.

themissingcow commented 4 years ago

Yes, they want to distinguish the kind of render. One person mentioned it doesn't necessarily need to be via the Type column, could be a custom column here instead (eg "Detailed Type"). But if it could be done via the main Type column, I'd be very happy.

If it's going to be in one column, I think it'll need multiple icons (which isn't a problem). Just that you have a matrix of render location and state. ie:

additionally, I would like to know if it's a local render or farm render.

So I guess the Q is @andrewkaufman @gregmassie-ie, is it local .vs. farm, or IPR vs one-shot, or both? Otherwise you could end up with three things to communicate.

Also, are you ever going to actually want to know more than just 'its on the farm' too? For example. you could just use the generic mechanism for the Host header, then you'd have a Compute Location column, which would even show you which machine it was running on, which might make interacting with Tractor etc easier? Not sure whats happening in net-render world at the mo and how that would work.

gregmassie-ie commented 4 years ago

@themissingcow I would think we'd want all of that info. :)

Local/Farm/IPR are all useful. And knowing which farm machine it's on is useful to gauge render cost. Martin here was also suggesting extracting Core Hours for renders into the catalogue to help better predict cost.

themissingcow commented 4 years ago

@themissingcow I would think we'd want all of that info. :)

Local/Farm/IPR are all useful. And knowing which farm machine it's on is useful to gauge render cost.

Cool, thanks @gregmassie-ie. So if Gaffer just says IPR or not, then you could easily add a column in your config with hostname.

Martin here was also suggesting extracting Core Hours for renders into the catalogue to help better predict cost.

I think right now, for a phase 1, It'll be limited to what data we get out of the image headers. Arnold does put a lot in there though. We could get into stats file parsing down the line, but it'd be great to keep the scope manageable for this first pass (the whole HUD type overlay is a separate topic all together that has had a lot of interest). We might be able to add in custom columns where you register a function so you could do that your end in the first instance. Would that work?

themissingcow commented 4 years ago

@andrewkaufman

It feels a bit unfortunate tbh... It means the column configuring is limited to script-saavy folks, and also means you can't easily configure per-catalogue.

John and I were just chatting this through, how about this for a middle ground:

This is something we could do in the current architecture without too much bother.

andrewkaufman commented 4 years ago

Local/Farm/IPR are all useful.

To clarify, for us, all 3 of these "modes" are wrapped inside a single Box along with a Catalogue

If it's going to be in one column, I think it'll need multiple icons (which isn't a problem). Just that you have a matrix of render location and state. ie:

Definitely, but if we're adding in cancelled state then we have to sort out this matrix anyways with the current design:

Maybe if we have a 'path / disk location' column we don't need the disk icon anymore, and the leading column becomes status/state rather than Type?

For example. you could just use the generic mechanism for the Host header... Not sure whats happening in net-render world at the mo and how that would work.

For us, net-render currently means a single farm blade doing the work, with the tiles coming back to the Catalogue display on your workstation (Farm mode above)... Maybe it'd have to be a Hosts column to account for some future multi-machine capability in Arnold.

andrewkaufman commented 4 years ago

the Meta tab for phase 1

The idea for phase one was that it would probably be a very basic pseudo-inspector, so you can at least see what header fields are there and their values without having to print them in the python editor, to aid with column configuration and general inspection.

Oh, were you thinking it'd only show "header:*" metadata? I think that'd be a bit confusing for people to not see the full metadata there.

Phase 1 was really trying to get the low hanging fruit, and it can flesh out over time, unless it's significantly less useful without.

Personally I'd prioritize metadata inspection features as follows:

  1. Display the key/values in a listing similar to Globals in the SceneInspector
  2. Make the listing filterable
  3. Make each key and/or value drag/droppable
  4. Diff visualizations

I'd suggest doing a quick mockup and testing with a few production images do decide which features are essential and which can be left for later.

At IE, we currently get 12 entries for an IPR image, so that's probably usable with just (1). But then you'll want to write a config to expose one of them as a column, and you can't drag/drop the key, so you're back to the PythonEditor unless we implement (3).

You can also load images into the Catalogue via the browse button (though admittedly I don't know how often that's done in practice). I picked 2 exrs at random here; an Arnold render has 71 metadata entries and a simple slap comp from an ImageWriter has 174 metadata entries. Based on those, I'd say for browsed images (2) is essential to make the metadata tab usable in production.

(4) is complex enough that I think it would be ok to leave for the future.

Side notes:

themissingcow commented 4 years ago

Thanks for the details Andrew, makes sense.

Oh, were you thinking it'd only show "header:*" metadata? I think that'd be a bit confusing for people to not see the full metadata there.

Sorry I just meant roughly what was in image["out"].metadata() plus obvious simple additions, as thats the thing that there is zero visibility of right now.

  • It seems a bit odd that IPR doesn't get all that detailed metadata from Arnold... I'd imagine once people can see the metadata more easily, they'll start asking for all the standard arnold data to come through Gaffer's display driver as well. If that happens, that's another vote for (2).

Yeah, I'd noticed that too, its on my list to work out how we can get the same via the socket display.

themissingcow commented 4 years ago

Catalog Phase 1 v2

Revised design that focuses on the Type/Status topic. Ignore the specifics of whether its one column or two for now. I wanted to try and capture what exactly we'd try to depict.

andrewkaufman commented 4 years ago

I like it, and I think the 2 columns makes sense now we have so much information to convey. My only concern would be if the :recycle: type icon will mislead people into thinking its a status rather than a type

themissingcow commented 4 years ago

I like it, and I think the 2 columns makes sense now we have so much information to convey. My only concern would be if the ♻️ type icon will mislead people into thinking its a status rather than a type

Cool, yeah, trying to think of icons for IPR and batch that are both off the same style and not 'updatey' isn't easy. We can refine those as we go if we're generally happy with the matrix of possible states?

johnhaddon commented 4 years ago

Thanks for all the feedback on this and the other design issues folks, it's all really valuable. I just want to make a general comment that while it's great to design our end goal, we'll actually be aiming to deliver is a series of incremental releases that move us towards our various goals, rather than hold everything up waiting for every last detail to be implemented. To take a specific example from this design, I'm not sure that it's possible for Gaffer to reliably distinguish between errored/cancelled for a batch render, so we will likely make an initial release with a more limited set of statuses. It might be that things like errored/cancelled/host are better implemented via custom extensions where you have access to the specifics of your farm setup.

themissingcow commented 4 years ago

I've updated the main Issue image to factor in all the various discussion.

themissingcow commented 4 years ago

@andrewkaufman @gregmassie-ie @murrays-ie

One of the refinements we've made during implementation is combining the type/status columns into one. Any thoughts on some potential icon options below:

catalogueIconsPreview

andrewkaufman commented 4 years ago

For IPR I think I prefer D-Running, but D-stopped is a bit confusing. Any objection to closing the circle?

For Batch, either seems fine by me.

I'll ask some other folks though.

murrays-ie commented 4 years ago

Splitting hairs here, but if these are going to be fairly small, for Batch I think B is better for clarity. For IPR, only C speaks "running" to me, the rest all feel like "refresh" or "restart" icons.

+1 on Andrews's comment about having more visual distinction between running and stopped.

gregmassie-ie commented 4 years ago

I'm with Murray. B and C.

Will C be animated while it's running for clarity... ;)

On Wed, 25 Mar 2020 at 14:43, Murray Stevenson notifications@github.com wrote:

Splitting hairs here, but if these are going to be fairly small, for Batch I think B is better for clarity. For IPR, only C speaks "running" to me, the rest all feel like "refresh" or "restart" icons.

+1 on Andrews's comment about having more visual distinction between running and stopped.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/GafferHQ/gaffer/issues/3646#issuecomment-604103635, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJP2V5CLXTLMQU2BBRUASLRJJ3INANCNFSM4KZHFLNQ .

-- Greg Massie - Lighting Supervisor

Image Engine studio: +1-604-874-5634 <1-604-874-5634> | cell: +1-604-417-6120 gregm@image-engine.com | www.image-engine.com

http://www.image-engine.com http://www.facebook.com/imageengine https://twitter.com/ImageEngine http://www.linkedin.com/company/image-engine-design-inc. http://vimeo.com/imageengine

15 West 5th Avenue, Vancouver, BC, V5Y 1H4, Canada

If you are not the intended recipient, disclosure, copying, distribution and use of this email is prohibited. Please notify us immediately and delete this email from your systems. You may contact us at info@image-engine.com info@image-engine.com?Subject=Please0Unsubscribe0me if you do not wish to receive further commercial electronic messages. We may still send you messages for which we do not require consent.

johnhaddon commented 4 years ago

Didn't we already get feedback that the arrows on C are unreadable at the size they'll actually be presented?

gregmassie-ie commented 4 years ago

For me the others do look like refresh buttons, as Murray mentioned. The circle represents continuity to me.

What if we based IPR icon on the infinity symbol? https://bit.ly/3dA7KE6

On Thu, 26 Mar 2020 at 02:24, John Haddon notifications@github.com wrote:

Didn't we already get feedback that the arrows on C are unreadable at the size they'll actually be presented?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/GafferHQ/gaffer/issues/3646#issuecomment-604320028, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJP2V345VRD3ZY4VW4SVBDRJMNL3ANCNFSM4KZHFLNQ .

-- Greg Massie - Lighting Supervisor

Image Engine studio: +1-604-874-5634 <1-604-874-5634> | cell: +1-604-417-6120 gregm@image-engine.com | www.image-engine.com

http://www.image-engine.com http://www.facebook.com/imageengine https://twitter.com/ImageEngine http://www.linkedin.com/company/image-engine-design-inc. http://vimeo.com/imageengine

15 West 5th Avenue, Vancouver, BC, V5Y 1H4, Canada

If you are not the intended recipient, disclosure, copying, distribution and use of this email is prohibited. Please notify us immediately and delete this email from your systems. You may contact us at info@image-engine.com info@image-engine.com?Subject=Please0Unsubscribe0me if you do not wish to receive further commercial electronic messages. We may still send you messages for which we do not require consent.