Support Ramp Widget For Read-Only Plugs #3417
Conversation
|
As mentioned, it'd be lovely to get this into 0.55 |
There was a problem hiding this comment.
Thanks for getting to the bottom of this one Daniel. I had already merged this when I realised we actually do have enough in the Slider API to proactively prevent rather than retroactively revert some of the unwanted edits, so let's take advantage of that where we can. I'll release as-is, but if you could make a follow up PR that would be great...
| @@ -155,22 +156,46 @@ def __positionsChanged( self, slider, reason ) : | |||
| if len( slider.getPositions() ) == plug.numPoints() : | |||
| # the user has moved an existing point on the slider | |||
| for index, position in enumerate( slider.getPositions() ) : | |||
| plug.pointXPlug( index ).setValue( position ) | |||
| if plug.pointXPlug( index ).getValue() != position : | |||
There was a problem hiding this comment.
I suspect the better approach to this would be to add something like a Slider.setPositionsReadOnly( boolPerPosition ) method, so we don't have to retroactively undo the unwanted actions, and the UI has the knowledge to represent immovability graphically if it wants to. One problem with that would be the inconsistent state in between a setPositions( someNewLength ) call and a setPositionsReadOnly( matchingTheNewLength ) call, so maybe it should really be a second argument to setPositions( positions, readOnly )? I dunno, I don't have a strong idea here, and @themissingcow and I are considering a future Slider refactor anyway, so I'm happy to leave that for later.
| plug.addPoint() | ||
| plug.pointXPlug( numPoints ).setValue( position ) | ||
| plug.pointYPlug( numPoints ).setValue( spline( position ) ) | ||
| if not ( plug.getInput() or plug.direction() == Gaffer.Plug.Direction.Out |
There was a problem hiding this comment.
This shouldn't be necessary - we already have Slider.setSizeEditable(), so we can just tell the slider upfront that it can't introduce new points, rather than having to retroactively remove them.
| plug = self.getPlug() | ||
| rejected = False | ||
| with Gaffer.UndoScope( plug.ancestor( Gaffer.ScriptNode ) ) : | ||
| if not ( plug.getInput() or plug.direction() == Gaffer.Plug.Direction.Out |
There was a problem hiding this comment.
This should also be covered by use of Slider.setSizeEditable().
| @@ -333,7 +333,7 @@ def __buttonPress( self, widget, event ) : | |||
| positions = self.getPositions()[:] | |||
| positions.append( float( event.line.p0.x ) / self.size().x ) | |||
| self._setPositionsInternal( positions, self.PositionChangedReason.IndexAdded ) | |||
| self.setSelectedIndex( len( positions ) - 1 ) | |||
| self.setSelectedIndex( len( self.getPositions() ) - 1 ) | |||
There was a problem hiding this comment.
We can revert this if we take advantage of setSizeEditable() instead.
It's been a long term annoyance for artists at IE that they would gang the channels of a color plug on a ramp, and everything would work fine, but then once they closed the ramp editor, they would be unable to open it again. We only recently figured out what was happening - it was just reported as "ramps break sometimes" - it is pretty tricky to figure out what's going on, because everything continues working fine until you close the window.
When thinking about how to fix this, one option would have been to allow showing ramps as long as everything but the Y components are editable, since the existing UI already handles readOnly y components properly.
I decided to take a quick look at fixing it properly though, so you can always open the editor, even for completely readOnly ramps. Maybe you'll spot something I've missed, but this now seems to all be working correctly, and is definitely a nice feature.