Menu

#1398 Add SetWhitespaceAlpha

Committed
closed
nobody
4
2021-06-24
2021-03-09
YX Hao
No

As dark theme is more and more popular, SetWhitespaceAlpha will make it easier to config 1 color with alpha that suites both light and dark colors.

Discussion

1 2 > >> (Page 1 of 2)
  • Neil Hodgson

    Neil Hodgson - 2021-03-09

    Do you mean the whitespace foreground or background (SCI_SETWHITESPACEFORE / SCI_SETWHITESPACEBACK) ?

    I expect you will see better results with appearance-sensitive colours. In SciTE, for example:

    if $(= $(Appearance);0)
        whitespace.fore=#444488
        whitespace.back=#FFFFA0
    if $(= $(Appearance);1)
        whitespace.fore=#FFAAAA
        whitespace.back=#000080
    
     
  • YX Hao

    YX Hao - 2021-03-10

    SCI_SETWHITESPACEFORE / SCI_SETWHITESPACEBACK, yes, related, defined in Scintilla.h saying generated from Scintilla.iface, the SetWhitespace*.

    I tried your example. It's appearance-sensitive. SciTE gives plenty customization switches, that awesome!

    There are other editors derived from scintilla. They focus on others features , but lack this feature. So I'm wondering, as a library, if there is a way that scintilla can implement, then all others will benefit from won't repeat it.

    I choose show white space for:
    1. Distinguish tab between blank chars to avoid a mixture.
    2. Align the code right.
    Anyway, codes are the focus. But unfortunately, some editors config the white space foreground too sensitive contrasting the background. There would be massive of them, if turned on. Then, I would always won't use the 'show Whitespace'.
    Tab is presented as arrow, sensitive enough. I prefer a low contrasting color for it.

    pseudo style code:

    size:2; alpha:25; fore:#FF4000
    

    Take a preview in html first. Inactive (Add //) the css body style line for light scheme.

    Like SetSelAlpha, SetWhitespaceAlpha can be implemented and called. Then 1 color with alpha for both scheme is possible.

    Specify foreground and background for both scheme is accurate. And add the alpha is another option, maybe easier to config.

     
    • Neil Hodgson

      Neil Hodgson - 2021-03-11

      OK, if someone contributes a good implementation, it can be included.

      However, since there are other ways to achieve reasonable behaviour, its not a priority for me.

       
  • Neil Hodgson

    Neil Hodgson - 2021-03-11
    • labels: --> scintilla, appearance
    • Group: Initial --> Won't_Implement
     
  • Neil Hodgson

    Neil Hodgson - 2021-03-11
    • Priority: 5 --> 4
     
  • YX Hao

    YX Hao - 2021-03-11

    Glad to contribute :)

    PS: patch file failed to be uploaded, trying zip file ...

     
    • Neil Hodgson

      Neil Hodgson - 2021-03-12

      The patch is showing significantly different results with single phase drawing and multi-phase drawing (in SciTE: phases.draw) as it is seeing different textBack in these modes. There are further issues with drawing on top of background indicators. Calculating a merged colour is a bit too simple.

      It would be better to use translucent drawing which will become available for lines in a 5.0.x release. That will merge the tab or space graphic onto the background that is actually there. AlphaRectangle or the SimpleAlphaRectangle wrapper can be tried to see what the effect is on spaces (or change DrawTabArrow to draw a rectangle).

      SC_ALPHA_NOALPHA is also something to avoid. It was introduced back when translucent drawing was not available on all platforms and as a way to avoid slower translucent calls when available. Most graphics APIs now include alpha in every function and SC_ALPHA_NOALPHA becomes a special case with extra code.

       
      • YX Hao

        YX Hao - 2021-03-13

        showing significantly different results with single phase drawing and multi-phase drawing

        I think you mean, for example:
        https://sourceforge.net/p/scintilla/code/ci/8585/tree/src/EditView.cxx#l1923 to l1931

                    ColourDesired textBack = TextBackground(model, vsDraw, ll, background, inSelection, inHotspot, styleMain, i);
                    if (ts.representation) {
                        if (ll->chars[i] == '\t') {
                            // Tab display
                            if (phasesDraw == phasesOne) {
                                if (drawWhitespaceBackground && vsDraw.WhiteSpaceVisible(inIndentation))
                                    textBack = vsDraw.whitespaceColours.back;
                                surface->FillRectangle(rcSegment, textBack);
                            }
        

        textBack changes.

        Maybe I haven't understood it completely, but I thought the opacity (alpha) for the foreground only is preferred, do you mean including the whitespaceColours.back too? That will be tricky to config for a better result.
        FillRectangle repaints the background with the new one. Foreground-opacity + CurrentBackground-opacity is more predictable. What do you think?

        If alpha is intended for both the foreground and background of white space, I think I'm going to treat the whole as a new image and try DrawRGBAImage.
        Thanks for any advice!

        BTW, SciTEDoc.html says:

        Single phase drawing (phases.draw=0) is a deprecated mode that was used in previous releases. While it can still be enabled, it is not supported and may cause incorrect drawing.

         

        Last edit: YX Hao 2021-03-13
  • YX Hao

    YX Hao - 2021-03-13

    Thanks for your comments.

    single phase drawing and multi-phase drawing (in SciTE: phases.draw)

    I'll study more about it.

    It would be better to use translucent drawing which will become available for lines in a 5.0.x release.

    Good idea for a more general new translucent drawing function. I assumed "the least changes".

    AlphaRectangle or the SimpleAlphaRectangle

    I tried, but the arrow head is oblique line. I will do more research on that ...

    SC_ALPHA_NOALPHA is also something to avoid

    Do you mean use SC_ALPHA_OPAQUE as default when ViewStyle::Init? That's different with other alpha defaults.

     
    • Neil Hodgson

      Neil Hodgson - 2021-03-13

      Here is an example with red whitespace background and with phases.draw=0 top or 1 (bottom).
      Translucent whitespace
      That can be 'fixed' with a little rearrangement for more consistent results but fixing that then leaves the background indicator issue.

      Do you mean use SC_ALPHA_OPAQUE as default when ViewStyle::Init? That's different with other alpha defaults.

      A new feature should duplicate current visuals unless an API is called to change the appearance.

      Maybe I haven't understood it completely, but I thought the opacity (alpha) for the foreground only is preferred, do you mean including the whitespaceColours.back too?

      It seems to me that WhitespaceAlpha applies to the foreground symbols (blocks and arrows), not the whitespace background as it is commonly drawn over nothing. The issue here is with drawIndicatorsBack and any new phases that are inserted between drawBack and drawText in the future. Perhaps the user has highlighted all tabs with a search for \t and a Mark All command. That will place indicators between the background and symbols. If the indicator is something like the following then using the whitespace background to define the apparent whitespace foreground may look incorrect.

      find.mark.indicator=style:roundbox,colour:#0080FF,under,outlinealpha:140,fillalpha:255
      

      BTW, SciTEDoc.html says:
      Single phase drawing (phases.draw=0) is a deprecated mode that was used in previous releases. While it can still be enabled, it is not supported and may cause incorrect drawing.

      That indicates that some features may not work in single phase drawing such as 'under' indicators. Its not a license to be incompatible when its not necessary.

       
      • YX Hao

        YX Hao - 2021-03-14

        Many thanks for your examples!
        I have compiled SciTE successfully and solved it to get the same alpha blended result. And choose clearer new variable names.
        See 'wsfa-markall-draw0.png', 'wsfa-markall-draw1.png', and 'Add-SCI_SETWHITESPACEFOREALPHA-and-SCI_GETWHITESPACEFOREALPHA.patch' in the zip file.

        For screenshots,
        SciTE instance: the left is official one with strong color; the right is patched one with weak color
        File opend: 1 is with phases.draw=0; 2 is with phases.draw=1

        highlighted all tabs with a search for \t and a Mark All command, then then marking indicators may look incorrect

        You're really expert to look so far to find potential issues!
        Yes, I reproduced it with phases.draw=0, but for the official release 5.0.0 too. Look at 'wsfa-markall-draw0.png'. I'm on Win10 with 32 bits version SciTE.
        This may be a new issue and should be fixed by another patch, I think.

        Please give me a review to see if I missed anything.


        BTW,
        1. I also added the patch and the test files for SciTE.
        2. All files zipped in one as only one attachment is possible.
        3. And extra things I found:
        3.1. If the whitespace.fore.alpha is adopted, as the foreground will become weak, should whitespace.size be implemented?
        3.2. The Appearance related. I set AppsUseLightTheme (used by SciTEWin::CurrentAppearance()), many apps don't change theme, including SciTE, colors still all are in light theme. Haven't got the reason yet. Most likely it's an issue of the OS. What I am wondering is: should there be an option to set different themes manually?

        If anyone is necessary to be logged in a new ticket, let me know.

        Thanks again for your detailed explanations to let me involve in. It make me learn a lot.

         
        • YX Hao

          YX Hao - 2021-03-18

          Hi Neil,

          I'm back on this ticket to upload an update for the patches and test files bundle.

          Based on the discussion https://github.com/zufuliu/notepad2/pull/298#issuecomment-801542554, I generated a new patch using simple alpha blending rather than AlphaRectangle or SimpleAlphaRectangle, in case of there are dense blanks (test file: 256000-blanks.zip). That will cause lagging if use GDI rendering (technology=0).

          I also amended the Implement whitespace.fore.alpha to be compact, and then added patch Implement whitespace.size (whitespace.size added in the test files).

          All previous files are included, and updated if necessary.

          Have a nice day!

           
  • Neil Hodgson

    Neil Hodgson - 2021-03-14

    There is a large set of platform layer patches pending from [feature-requests:#1364]. Applying this change will interfere in their incorporation so won't happen until after they have been processed. If you are interested, the 5.patch file contains most changes to Surface which can be found by searching for "--- a/include/Platform.h".

    3.1. If the whitespace.fore.alpha is adopted, as the foreground will become weak, should whitespace.size be implemented?

    Possibly.

    3.2. The Appearance related. I set AppsUseLightTheme (used by SciTEWin::CurrentAppearance()), many apps don't change theme, including SciTE, colors still all are in light theme.

    The standard Win32 controls don't really support appearance changing through Personalisation | Colours. It is better supported for .NET applications. What does change the appearance is changing to High Contrast (High Contrast Black) in Ease of Access settings. However, the high contrast themes are far too ugly to use unless needed because of sight problems.

    What I am wondering is: should there be an option to set different themes manually?

    Users seem more interested in harmonisation with system settings than in making SciTE different.

     

    Related

    Feature Requests: #1364

    • YX Hao

      YX Hao - 2021-03-15

      There is a large set of platform layer patches pending from [feature-requests:#1364]. Applying this change will interfere in their incorporation so won't happen until after they have been processed. If you are interested, the 5.patch file contains most changes to Surface which can be found by searching for "--- a/include/Platform.h".

      Okey, I'll have a try to see if I can help in anything.

      However, the high contrast themes are far too ugly to use unless needed because of sight problems.

      That's true!

       

      Related

      Feature Requests: #1364

    • YX Hao

      YX Hao - 2021-03-15

      Good news to me is I am already v5.0.0 based 😂. Hoping it won't conflict with others ...

       
  • Neil Hodgson

    Neil Hodgson - 2021-03-31

    The new APIs for translucent drawing have now been committed to the repository although not yet in a release.

     
    • YX Hao

      YX Hao - 2021-03-31

      I have amended and regenerated the 3 patches using the new ColourAlpha based drawing APIs. Locally tested.
      One known problem is: to legacy GDI, it would be CPU consuming when there are many blanks. Test files are in: test-wsfa\technology-0\.

       
  • Neil Hodgson

    Neil Hodgson - 2021-03-31

    whitespace.size has been added to SciTE as [02e7f5] but with a modified explanation.

    I don't understand why, in the code for space characters, the double test if ((phasesDraw == PhasesDraw::one) && drawWhitespaceBackground) was changed to two tests. The main effect of the change is to extract the assignment to textBack so it is performed even for multi-phase drawing, but that value is never read outside the single phase code.
    There is a similar concern in the tab drawing code. These changes may have been needed for an earlier version where textBack was mixed into textFore but those changes don't appear necessary now.

    There is a new API to make it easier to override colours without defining new APIs: SCI_SETELEMENTCOLOUR which is currently used to change autocompletion list colours. It may be better to define this feature as

    val SC_ELEMENT_WHITE_SPACE=10
    

    then use

    vs.ElementColour(SC_ELEMENT_WHITE_SPACE) -> std::optional<ColourAlpha>
    

    The current patch can change the alpha of visible whitespace when using the lexer defined colour (no SCI_SETWHITESPACEFORE override) which wouldn't be possible with SCI_SETELEMENTCOLOUR but that wasn't a goal in the initial post for this issue.

     

    Related

    Commit: [02e7f5]

    • YX Hao

      YX Hao - 2021-04-01

      if ((phasesDraw == PhasesDraw::one) && drawWhitespaceBackground) was changed to two tests

      You are right. Not necessary anymore, would be amended.

      SCI_SETELEMENTCOLOUR

      I noticed it. And it seems many existing 'SetColour' APIs, like SetWhitespaceFore, SetWhitespaceBack, SetSelFore and etc., can be replaced by reconstructing (This is beyond this topic).
      Thanks for your good example on how to use it, that explains my wonders clear.

      But I got issues if don't add new API to store the alpha and then get the final foreground ColourAlpha dynamically. Text foreground ColourAlpha would be preset solid, that would have issues on: only alpha is set, but without fore.
      Alpha blending with the current foreground would be better. What do you think?
      I took a screenshot with:

      whitespace.fore.alpha=153
      whitespace.size=4
      
       
    • YX Hao

      YX Hao - 2021-04-01

      Using SCI_SETELEMENTCOLOUR to set/store alpha only seems possible, but makes people confused ...

      Still haven't got an easier/better solution. Just update the current as attached.

      PS:
      hg transplant doesn't place the alpha setting right for SciTE, so the patch also updated.

       
  • Neil Hodgson

    Neil Hodgson - 2021-04-01

    I noticed it. And it seems many existing 'SetColour' APIs, like SetWhitespaceFore, SetWhitespaceBack, SetSelFore and etc., can be replaced

    Yes. There are a bunch of existing APIs that can be unified with SCI_SETELEMENTCOLOUR. Here's a list that appeared reasonable.

    val SC_ELEMENT_SELECTION=0
    val SC_ELEMENT_SELECTION_BACK=1
    val SC_ELEMENT_SELECTION_ADDITIONAL=2
    val SC_ELEMENT_SELECTION_ADDITIONAL_BACK=3
    val SC_ELEMENT_CARET=4
    val SC_ELEMENT_CARET_ADDITIONAL=5
    val SC_ELEMENT_CARET_LINE_BACK=10
    val SC_ELEMENT_FOLD_EDGE=20
    val SC_ELEMENT_HOT_SPOT_ACTIVE=30
    val SC_ELEMENT_HOT_SPOT_ACTIVE_BACK=31
    val SC_ELEMENT_WHITE_SPACE=40
    val SC_ELEMENT_WHITE_SPACE_BACK=41
    val SC_ELEMENT_FOLD_MARGIN=50
    val SC_ELEMENT_LIST=60
    val SC_ELEMENT_LIST_BACK=61
    val SC_ELEMENT_LIST_SELECTED=62
    val SC_ELEMENT_LIST_SELECTED_BACK=63
    val SC_ELEMENT_CALL_TIP=70
    val SC_ELEMENT_CALL_TIP_BACK=71
    val SC_ELEMENT_CALL_TIP_HIGHLIGHT=72
    

    A goal here is to simplify the API which is too large.

    But I got issues if don't add new API to store the alpha and then get the final foreground ColourAlpha dynamically. Text foreground ColourAlpha would be preset solid, that would have issues on: only alpha is set, but without fore.

    With element colours the code appears something like:

    const ColourAlpha whiteSpaceFore =
        vsDraw.ElementColour(SC_ELEMENT_WHITE_SPACE).value_or(textFore);
    

    textFore is a ColourDesired which is converted into an opaque ColourAlpha in this context.

    Adding alpha for all visible whitespace is an extra feature that I feel is an overcomplication. Scintilla has a large number of settings, many of which are rarely or never used. I expect most people that want visible whitespace will want it to be fully opaque to be most visible and if they define a colour it will be one that makes sense in their current colour scheme.

     
    • YX Hao

      YX Hao - 2021-04-02

      An email ping of attachment updated.

       
  • YX Hao

    YX Hao - 2021-04-02

    a bunch of existing APIs that can be unified

    Seems all on your list 👍

    people that want visible whitespace will want it to be fully opaque to be most visible

    Here we just give users an option, making it more customizable. Of course, with serveral lines of code, rather than heavy code. Use or not, or use which one, all depend on themselves.
    To me, making visible whitespace the 2nd attractive and code always the 1st attractive is really appreciated.

    const ColourAlpha whiteSpaceFore = vsDraw.ElementColour(SC_ELEMENT_WHITE_SPACE).value_or(textFore);

    That is what I want, and inspired me, making things simpler. Appreciate!

    Finally, I made it, with only one more val SC_ELEMENT_WHITE_SPACE_ALPHA_ONLY=42 added, to cover the case "only alpha is set" and distinguish it from explicit whitespace.fore=#000000.

    Edit:
    Update attachment.

     

    Last edit: YX Hao 2021-04-02
    • Neil Hodgson

      Neil Hodgson - 2021-04-06

      Finally, I made it, with only one more val SC_ELEMENT_WHITE_SPACE_ALPHA_ONLY=42 added, to cover the case "only alpha is set" and distinguish it from explicit whitespace.fore=#000000.

      Seems to me, this is reducing the clarity that is the goal of SCI_SETELEMENTCOLOUR. Its supposed to be an easy way for a developer to change the colour of something with translucency a bonus if wanted. Using an extra element for "just translucency" then combining that with other settings produces a more complex mental model. Going back to an explicit SCI_SETWHITESPACEFOREALPHA when its affecting text colours is simpler. Then the question is: does the SCI_SETWHITESPACEFOREALPHA affect SC_ELEMENT_WHITE_SPACE? I'd say no - SCI_SETELEMENTCOLOURoverrides the colour of the whitespace so it should also override the alpha.

      Testing against SC_ALPHA_NOALPHA in the v5 patch shouldn't be needed. SC_ALPHA_NOALPHA will be deprecated and eventually go away.

      ColourAlpha textColourAlpha = ColourAlpha(textFore);
      if (vsDraw.whitespaceForeAlpha != SC_ALPHA_NOALPHA)
          textColourAlpha = ColourAlpha(textFore, vsDraw.whitespaceForeAlpha);
      

      is simpler as

      const ColourAlpha textColourAlpha(textFore,
          vsDraw.whitespaceForeAlpha);
      
       
  • YX Hao

    YX Hao - 2021-04-02

    Here is an update for the multiple options files to work well.

    OptionalSetColour(wEditor, SA::Element::WhiteSpace, props, "whitespace.fore"); added, but SetWhitespaceFore not removed.

    The more I read, the more I learnt. ❤👍🌹

     
1 2 > >> (Page 1 of 2)

Log in to post a comment.