sublimehq / sublime_text

Issue tracker for Sublime Text
https://www.sublimetext.com
810 stars 39 forks source link

`view.style_for_scope` does not include `background` #5942

Open rchl opened 1 year ago

rchl commented 1 year ago

Description of the bug

-

Steps to reproduce

  1. Customize color scheme with:

    {
    "rules": [
        {
            "scope": "markup.unnecessary.lsp",
            "foreground": "color(rgb(255, 255, 255) alpha(0.4))",
            "background": "color(var(blue3) alpha(0.9))"
        },
    ],
    }
  2. Run in the console:

    'background' in view.style_for_scope('markup.unnecessary.lsp')

Expected behavior

True

Actual behavior

False

Sublime Text build number

4148

Operating system & version

macOS

(Linux) Desktop environment and/or window manager

No response

Additional information

Works in latest ST4 stable.

OpenGL context information

No response

deathaxe commented 1 year ago

Applying provided rule to Mariana and calling the api returns valid background in ST4148 on Win11.

>>> view.style_for_scope('markup.unnecessary.lsp')
{'background': '#303841', 'bold': False, 'foreground': '#82878d', 'italic': False, 'source_column': 26, 'source_file': 'Packages/User/Mariana.sublime-color-scheme', 'source_line': 4}

If the active color scheme however does not provide the variable blue3, used by background rule, it becomes invalid and thus not returned by api:

>>> view.style_for_scope('markup.unnecessary.lsp')
{'bold': False, 'foreground': '#707276', 'italic': False, 'source_column': 26, 'source_file': 'Packages/User/Brackets Dark.sublime-color-scheme', 'source_line': 30}
rchl commented 1 year ago

On Mac it fails in safe mode with Mariana:

Screenshot 2023-04-19 at 00 24 56

rchl commented 1 year ago

It does work with "background": "#000" but not with "background": "var(blue3)". So as you say, it seems to be related to resolving the reference to blue3 but the variable is present in the original color scheme file at least. So maybe some issue with merging scopes.

jwortmann commented 1 year ago

I think this is somewhat expected, see the docs for style_for_scope: "background" (only if set)

It seems that "only if set" here actually means "only if different from regular background", so the docs are a little bit inaccurate in this point. Note that the regular background in Mariana is var(blue3), so if you overlay var(blue3) with an alpha of 0.9, it is still the same color var(blue3). You can confirm by using

        {
            "scope": "markup.unnecessary.lsp",
            "foreground": "color(rgb(255, 255, 255) alpha(0.4))",
            "background": "color(var(blue3) alpha(1.0))"    // or "background": "#303841"
        },

on latest ST stable, and see that in this case "background" isn't included in the result in latest ST stable too.

Why latest stable does include the "background" when you reduce the alpha value, I can't say, but I could imagine this to be some kind of rounding error or floating point comparison inaccuracy (I speculate that internally the colors are converted to HSL or so, and I have noticed very small color inaccuracies before, compared to the exact analytic values, when using transparency).

And maybe this was fixed in the latest dev build, which would be a good thing.

(well, it still wouldn't explain the discrepancy between Mac and Windows, but my point is that you apparently cannot rely on "background" being present if you don't change the actual color value)

BenjaminSchaaf commented 1 year ago

These colors are floating point internally, so using a different method of creating the same color is likely to result in slight differences. Indeed the background color is ignored if equivalent to the color scheme's background. I don't think the functionality here is wrong.

rchl commented 1 year ago

How is that a documentation issue?

Color:

"background": "color(var(blue3) alpha(0.9))"

is not the same as:

"background": "color(var(blue3))"

Why would the engine think that those are equivalent in latest dev builds?

jwortmann commented 1 year ago

I agree that it is only a documentation issue. @rchl have you read the second paragraph in my comment? ST compares the computed values and not the string values from the color scheme. For example would you argument that red, rgb (255, 0, 0), #FF0000 are all different?

rchl commented 1 year ago

Your second paragraph talks about values color(var(blue3) alpha(1.0)) and color(var(blue3)) which I agree that those would evaluate to the same value but I'm talking about the case color(var(blue3) alpha(0.9)) and color(var(blue3)). The difference being a different alpha value.

jwortmann commented 1 year ago

But color(var(blue3) alpha(0.9)) is still evaluated against the background of var(blue3), so the resulting color is still the same. There is no need to store an alpha channel in the resulting color, because the Sublime Text window is opaque.

rchl commented 1 year ago

Aren't we going a bit too deep into low-level intricacies of the engine?

High level wise, the color is different. It looks different to the user so why should it not be returned by style_for_scope? Otherwise I don't know if it's at default value or customized.

In CSS rgb(1, 1, 1) is also not the same as rgba(1, 1, 1, .3).

BenjaminSchaaf commented 1 year ago

@rchl it's my understanding that the color doesn't look different. If you mix blue3 into blue3 using any alpha value you want you'd still get blue3 as a result. This is also what I observed testing this: No background is shown because the color matches the background.

jwortmann commented 1 year ago

so why should it not be returned by style_for_scope? Otherwise I don't know if it's at default value or customized.

I think from the discussion here, the actual reason for this issue is a bit misleading. The behavior of View.style_for_scope should be fine as it is now. I guess what @rchl and myself too would actually prefer to have, is a way to directly adjust the foreground color, in the way described in this comment: https://github.com/sublimehq/sublime_text/issues/817#issuecomment-691114174.

I don't know how difficult it would be to implement, but ST already has the foreground dimming built-in, to draw control characters (see my screenshot in https://github.com/sublimelsp/LSP/issues/1227#issuecomment-857798448). This functionality is just not exposed in the plugin API.

rchl commented 1 year ago

@rchl it's my understanding that the color doesn't look different. If you mix blue3 into blue3 using any alpha value you want you'd still get blue3 as a result. This is also what I observed testing this: No background is shown because the color matches the background.

You are right. The background color actually doesn't change. Not sure why but it's not new behavior at least.

Well, then I give up :)

And yes, being able to change the foreground color without having to do weird hacks with setting background would be the ultimate goal.