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.

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

Customizable column color filters #4756

Merged

Conversation

shun-iwasawa
Copy link
Member

This PR will enable to customize column color filters via the Scene Settings popup.
image

Please note that once the settings have altered from the default, such scene won't be opened from the previous versions of OT before this PR.

@RodneyBaker
Copy link
Collaborator

@shun-iwasawa This may appear to be a small change but in my estimation it's huge.
You've anticipated a need that has been there since Opentoonz was first released and delivered it in an even better way that I anticipated.

A quick test suggests everything is working as advertised.

Thanks also for the mention of the breaking change as users will definitely want to know that.

@RodneyBaker
Copy link
Collaborator

@shun-iwasawa A question...

While we can set matte/alpha transparency in the custom color, from what I can tell this has no effect on the column filter.
That is reserved for the opacity slider only at this time, correct?

@shun-iwasawa
Copy link
Member Author

@RodneyBaker Thank you for reviewing the feature!

While we can set matte/alpha transparency in the custom color, from what I can tell this has no effect on the column filter.
That is reserved for the opacity slider only at this time, correct?

That was correct, but there may be an advantage to including the alpha channel of the color filter in the column opacity calculation. I'll try to modify it a bit. Thanks for the idea!

@RodneyBaker
Copy link
Collaborator

RodneyBaker commented Mar 1, 2023

Thanks @shun-iwasawa I could have answered my own question in looking at the code but am always more interested in your answers! :)

I did see that the alpha value is being stored in the scene file and that got me a little excited about the possibilities.
There seems to be a number of possibilities. For example:

  • The alpha value and the slider value could be connected so they are actually the same thing
  • The alpha value and the slider value could be some form of Min/Max that counter each others values
  • The alpha value and the slider value might add or subtract from each other (oerhaps same general approach as Min/Max above)
  • The slider value could add to or multiply the value of the underlying alpha (floor of zero and capped at 255 presumably but... not sure how that might work with 32 bit float images)
  • The alpha might be an actual punch/mask/matte that cuts through the images contained in columns below the current column
  • Some other thing not captured here which I'm sure you'll have considered and will implement.

The customizable column filter colors are a great addition as is and others have already responded with positive feedback regarding this PR. That it might be plussed up a little more is just icing on the cake. :)

@RodneyBaker RodneyBaker mentioned this pull request Mar 1, 2023
3 tasks
@shun-iwasawa
Copy link
Member Author

The opacity of the displayed column is now the alpha channel of the column's color filter multiplied by the value of the column's opacity slider.

@RodneyBaker
Copy link
Collaborator

That's got it. Nice! Thanks @shun-iwasawa

The only thing I'm having difficulty with currently is setting colors to either fully white or fully black.
RGB values of 0,0,0 and 255,255,255 appear to me as potentially problematic.

In the case of white it seems that no matter then change to opacity or slider the resulting output is... completely white.
Perhaps that's as it should be but it seems odd. Perhaps the opacity is being ignored at that value?

With black I'm finding that I cannot set complete black (0,0,0) which would make sense I suppose as well.
If changing just one value in any RGB channel (or say... using a value of 1,1,1 we can get the full spectrum from white (white replaces original image) to fully transparent (i.e. the image appearing as if there was no color tinting at all).

If someone could test full white and full black and observe the results that will be appreciated.
(Note that use of pure white was also an issue prior to Shun's latest modification. I can't recall testing pure black.)

@RodneyBaker
Copy link
Collaborator

Here is an example of a frankenstein'd reverse color separation thingy:

All the images are black and white/grayscale with the color provided by Custom Column Color Filters (tm).

image

@RodneyBaker
Copy link
Collaborator

Will press on with this PR and get it into the hands of Opentoonz users.

Thank you @shun-iwasawa for the customizable column color filters.
LGTM

@RodneyBaker RodneyBaker merged commit d08b536 into opentoonz:master Mar 2, 2023
@RodneyBaker
Copy link
Collaborator

RodneyBaker commented Mar 2, 2023

I meant to post a follow up to the (percieved) black and white colors issue I posted before.

That might have been related to the build I was testing.
I don't see any issues with black and white in the appveyor and nightly build.

Edit: I might have spoken too soon. Still testing.

@RodneyBaker
Copy link
Collaborator

RodneyBaker commented Mar 2, 2023

I did confirm that scenes created with adjusted column color will not open in earlier versions.
It will be good to consider this when troubleshooting.

Only black style No.1 used in the following:

customcolumnfilter animated

Project file:
CustomColumnColor animated.zip

@RodneyBaker
Copy link
Collaborator

RodneyBaker commented Mar 18, 2023

@shun-iwasawa Just a heads up that there seems to be an issue with Column color filters not displaying in regular/real time view. The colors appear in Preview only. I haven't posted a bug report yet as still trying to figure out what we are seeing.

Edit: I cannot seem to consistently reproduce the problem.

@RodneyBaker
Copy link
Collaborator

This is to note the issue reporting column tinting is now resolved.
REF: #4803

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants