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

Brush engine value/lightness variation produce incorrect result. #1069

Closed
ChengduLittleA opened this issue Apr 9, 2020 · 8 comments
Closed

Comments

@ChengduLittleA
Copy link

Hi! I just found a problem with new pigment mode.

Description of the problem

When in 2.x capability mode (pigment), "HSL/HSV value change modifier" in opaque brushes behave very badly in low brightness colours, the result colour brightness jumps too much while in settings the variation is set at 0.01 or 0.02. if I switch to 1.x mode and create new file, the brush works as expected. This would render the brush useless in actual painting sessions.

I believe it's a bug with brush engine, i tried the following,:

  • Set brush "pigment" value to 0.
  • Use brush on "normal" instead of "pigment" layer.

and neither of the steps solves the above problem unless i use 1.x capability mode and create new file. I don't want to do that because I still find pigment blending in brushes very useful. the Only problem I had is that the "value/lightness variation" is not correctly interpolated.

This might be caused by liner processing of the colour data, but in this kind of use what we do care is its visual appearance.

Basic system details

MyPaint version: 2.1.0-alpha+git.984ded6a (I compiled and installed mypaint and libmypaint)
Operating system: Ubuntu 18.04LTS
Desktop environment: XFCE

Steps to reproduce

  1. Start MyPaint (in 2.x mode)
  2. Use a brush (default opaque acrylic brush will do, if it's not there use any simple brush and go to brush's settings panel->color->HSL or HSV changes, set "stroke" or "random" entry to 0.02 or 0.01)
  3. set colour to about 1/10 of full brightness.
  4. paint and observe the value/lightness variation. (the brightness jumps around quite a bit)
  5. use 1.x mode, create new file, then the same step would produce reasonable subtle variation. (which is about the same percentage of set value)
  6. layer mixing mode and brush pigment slider doesn't help.

Backtraces or error messages

Doesn't have any.


I'll try to see if there's anything I can do within libmypaint, maybe I'll provide a patch to it. But dear developers please see to this issue, it's quite important as for artistic needs.

Thanks in advance!

@jplloyd
Copy link
Member

jplloyd commented Apr 9, 2020

Confirmed the behavior of the lightness/value mappings.

One workaround when working with dark colors is to change the domain of the input mappings to move the negative component closer to 0, but that's not very convenient if you want to use the same brush for brighter colors as well.

@briend
You think it's feasible to compensate for this in libmypaint?

@ChengduLittleA
Copy link
Author

Hello! I just did some researches on the code and found it's actually a linear/sRGB problem within the HSL modifier. When in 2.x mode, the modifier doesn't take account of the non-linear input color, it still maps linearly, result in excessive changes in low brightness colors.

I did some digging, and found the EOTF (2.2 gamma) doesn't seem to be accessible in C code, and the brush color is pre-trasformed in python and then given too the brush engine. the brush engine doesn't seem to have the information whether it's running in sRGB or Linear mode, which the configuration is also saved in a python structure.

In order to paint like I used to, I did a small hack in HSL modifier, to convert colors back before applying changes, but it's a hard-coded gamma, and I'm using the pigment(paint) field to control whether I want linear or sRGB transformed back.(which currently allows me to work on old files s well)

My suggestion would be making color mode information accessible in C code in the brush engine, or to store another "raw" color somewhere in SETTINGS() or BASEVAL().

Also, thanks for the timely confirmation :D

@jplloyd
Copy link
Member

jplloyd commented Apr 10, 2020

You are correct on all points. To solve this properly we need to supply information about the EOTF to libmypaint. Your hack is exactly the same stopgap measure I had in mind: https://irclogs.jackgrigg.com/irc.freenode.net/mypaint/2020-04-09#i_4554066

As I wrote there, it messes with the following situations:

  1. For 2.x mode: Brushes w. pigment turned off manually, using any color-changing setting.
  2. For 1.x mode: Brushes w. pigment turned on manually, using any color-changing setting.

I think the most common scenario for 1 is using a sketch or fill brush, which usually would not make use of color-changing. For 2, enabling pigment mode in 1.x mode is already not recommended since it doesn't function as well. In other words, I think the stopgap measure would be an ok "solution".

An alternative simple (but annoying) solution is to add a new mypaint_brush_stroke_to_3 that handles the linear case, create a libmypaint 1.6 release and call either stroke_to_2 or stroke_to3 from MyPaint depending on the EOTF.

@ChengduLittleA
Copy link
Author

Thanks for the information :D

I'm gonna continuing using my hacks until some more solid solution comes up. You are right, older files doesn't really want pigment mode anyway, so if new files behave correctly, I think there's no necessity of doing a bunch of complicated compatibility stuff in the brush engine. Also, there's only a few modifiers that changes color in the brush engine.

I'm not quite familiar about how to pass python data (i believe in this case data is passed through json-c) to libmypaint, but anything pointing me to the direction would be appreciated :D I'm glad to help out.

@jplloyd
Copy link
Member

jplloyd commented Apr 10, 2020

Using json directly is not actually part of the callchain at all, although I wouldn't blame anyone for thinking that it is.

Call setup is roughly this:

Functions/methods in C++ files (in mypaint/lib/) make calls to libmypaint interface functions.

The public functions/classes/methods in the C++ files are fed to swig, which generates python-accessible wrappers compiled to a shared library, and an associated python module (mypaintlib.py / _mypaintlib.so).

The functions/classes in mypaintlib.py are then called/constructed from the regular python modules.


To make the proper(ish) fix yourself, you can pass either a flag or the gamma value to a modified version of mypaint_brush_stroke_to_2 by adding a corresponding parameter to Brush::stroke_to in lib/brush.hpp and PythonBrush::stroke_to in lib/python_brush.hpp. Then you would need to either update all (11) stroke_to calls to pass the new argument (be it the gamma or a flag) or (more sensibly) override the stroke_to method in class Brush in lib/brush.py and grab the eotf from lib/eotf.py.

Then you would have to modify the functions in mypaint-brush.c to only apply the transformations conditionally.

@ChengduLittleA
Copy link
Author

Thank you for the clarification :D I'm taking a look.

@briend
Copy link
Contributor

briend commented Apr 10, 2020

The more I think about the more I really think libmypaint should be entirely unconcerned and unaware of color models or encodings. It very nearly is besides these HSV/HSL color settings and the colorize brush mode. If we could strip these out and maybe implement alternatives on the MyPaint UI side of things I think we'd be better off and libmypaint would be more flexible for everyone.

That is, it is a pretty small leap to allow libmypaint to process any kind of data; sRGB, linear RGB, log data, and even expand the color channels to >3 as I've done in my personal branch. As long as libmypaint does "color" stuff, it'll be increasingly difficult and cpu intensive. Imagine handling 10 channels of log-encoded data; to fiddle with the color in libmypaint you'd have to round-trip from 10 channel log -> linear -> XYZ (using a 10x3 matrix) to RGB to HSV and then go back to 10 channels (this would lose metameric color information along the way).

The colorize brush mode could largely be replaced by just using a layer mode (perhaps we could create temporary layers even like other programs do).

Having ways to fiddle with the brush color could be implemented on the MyPaint UI side pretty simply and might be more useful as you could do extra things like use your palette or color history as sources of color data, or even your microphone input (silly, but Corel Painter has this).

jplloyd added a commit that referenced this issue Apr 29, 2020
Fixes the incorrect colors in issue #1069
Bumps libmypaint requirement to >= 1.6
@ChengduLittleA
Copy link
Author

Thanks for the fix.

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

No branches or pull requests

3 participants