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

Regression, theming inconsistency in 190c #1024

Closed
hollunder opened this issue Nov 24, 2019 · 74 comments
Closed

Regression, theming inconsistency in 190c #1024

hollunder opened this issue Nov 24, 2019 · 74 comments
Assignees
Milestone

Comments

@hollunder
Copy link

Hi Paul,
I just noticed something weird. Have a look at these screenshots.

2019-11-24-004528_3840x1080_scrot
2019-11-24-005445_3840x1080_scrot

The second one where it's just 1.80 is how I'd expect it to look. This one was built using this script: zerobrane-studio.
They all use the same wxgtk/wxlua (my build from git).
What's really weird is that it seems to me that only the inner parts of zbstudio are so ugly while the rest looks like it is adhering to the gtk2 theme I have set.

@pkulchenko
Copy link
Owner

@hollunder, what inconsistency are we looking at here? The tabs in the notebook? There was a change in wxwidgets to reset the default configuration, so I had to enforce a specific style for the notebook, but from the documentation it should match what was there earlier.

Can you try commenting out the line 742 (nb:SetArtProvider(wxaui.wxAuiGenericTabArt())) in src/editor/gui.lua and check if it matches the "old" look? If you are referring to something else, please be specific.

@pkulchenko pkulchenko self-assigned this Nov 24, 2019
@hollunder
Copy link
Author

Yes, I was referring to the tabs and their background. I didn't notice it earlier because on my laptop I use a bright theme and there it looks fine. It's apparently only with a dark theme (Arc-Dark-solid in this case) where it falls apart (extreme gloss, barely readable font).

For me that file has only 739 lines and does not contain said line.
However, I took a pot shot and commented line 735 nb:SetArtProvider(ide:GetTabArt()) and that got rid of the problem.
2019-11-24-084609_3840x1080_scrot

Btw. because GTK3 is mentioned in the source comment there, does it work with GTK3 and do you recommend 3 over 2?

@pkulchenko
Copy link
Owner

Yes, I tested with GTK3 and didn't see any difference; I didn't realize that this is going to be different with GTK2. I think I may just have disable this for Linux to keep it consistent with how it's set as the default.

@pkulchenko
Copy link
Owner

Actually, can you try with GTK3 to see if there is any difference with the result?

@hollunder
Copy link
Author

hollunder commented Nov 26, 2019

I just tried with GTK3, by setting wx-config-gtk3, which seems to have worked: -- * - wxWidgets_LIBRARIES = -pthread;;;-lwx_gtk3u_stc-3.0;-lwx_gtk3u_gl-3.0;-lwx_gtk3u_html-3.0;-lwx_gtk3u_aui-3.0;-lwx_gtk3u_adv-3.0;-lwx_gtk3u_core-3.0;-lwx_baseu_net-3.0;-lwx_baseu-3.0

This is on my laptop where I don't have the same dark theme installed, so I picked a different dark theme, so it doesn't look identical but the issue is still clearly there.

2019-11-26-082326_1366x768_scrot

Just for reference, it looks fine with my usual bright theme.
2019-11-26-083056_1366x768_scrot

@pkulchenko
Copy link
Owner

This is something that needs to be addressed in wxwidgets; I opened a ticket for it.

@pkulchenko
Copy link
Owner

@hollunder, could you report back the numbers you get for that back theme when you execute the following commands in the Console?

wx.wxSystemSettings.GetColour(wx.wxSYS_COLOUR_3DFACE):GetAsString()
wx.wxSystemSettings.GetColour(wx.wxSYS_COLOUR_CAPTIONTEXT):GetAsString()
wx.wxSystemSettings.GetColour(wx.wxSYS_COLOUR_INACTIVECAPTIONTEXT):GetAsString()

@hollunder
Copy link
Author

For the Breeze-Dark theme, a grey-ish one.

wx.wxSystemSettings.GetColour(wx.wxSYS_COLOUR_3DFACE):GetAsString()
"rgb(49, 54, 59)"
wx.wxSystemSettings.GetColour(wx.wxSYS_COLOUR_CAPTIONTEXT):GetAsString()
"rgb(239, 240, 241)"
wx.wxSystemSettings.GetColour(wx.wxSYS_COLOUR_INACTIVECAPTIONTEXT):GetAsString()
"rgb(127, 140, 141)"

For deepin-dark, a almost black theme.

wx.wxSystemSettings.GetColour(wx.wxSYS_COLOUR_3DFACE):GetAsString()
"rgb(27, 30, 32)"
wx.wxSystemSettings.GetColour(wx.wxSYS_COLOUR_CAPTIONTEXT):GetAsString()
"rgb(137, 145, 154)"
wx.wxSystemSettings.GetColour(wx.wxSYS_COLOUR_INACTIVECAPTIONTEXT):GetAsString()
"rgb(137, 145, 154)"

These are two I have installed on the laptop, I can send you the colours for the one I'm actually using on the desktop in a couple hours.

@hollunder
Copy link
Author

For the theme I use on my desktop, Arc-Dark-Solid, these are the colours.

wx.wxSystemSettings.GetColour(wx.wxSYS_COLOUR_3DFACE):GetAsString()
"rgb(56, 60, 74)"
wx.wxSystemSettings.GetColour(wx.wxSYS_COLOUR_CAPTIONTEXT):GetAsString()
"rgb(211, 218, 227)"
wx.wxSystemSettings.GetColour(wx.wxSYS_COLOUR_INACTIVECAPTIONTEXT):GetAsString()
"rgb(211, 218, 227)"

@pkulchenko
Copy link
Owner

@hollunder, thank you for the details. Can you apply the following patch to wxwidgets and recompile it? I'm not sure if it's going to apply it cleanly to your version of wxwidgets (3.0.4?), but it should explicitly set the font color in DrawTab method of wxAuiGenericTabArt. You can ignore the first fragment if it doesn't patch cleanly.

diff --git a/src/aui/tabart.cpp b/src/aui/tabart.cpp
index 7c512f95d7..c659534568 100644
--- a/src/aui/tabart.cpp
+++ b/src/aui/tabart.cpp
@@ -267,14 +267,6 @@ void wxAuiGenericTabArt::DrawBackground(wxDC& dc,
     // draw background
     int topLightness = 90;
     int bottomLightness = 170;
-    if ((m_baseColour.Red() < 75)
-        && (m_baseColour.Green() < 75)
-        && (m_baseColour.Blue() < 75))
-    {
-        //dark mode, we cannot go very light
-        topLightness = 90;
-        bottomLightness = 110;
-    }
 
     wxColor top_color    = m_baseColour.ChangeLightness(topLightness);
     wxColor bottom_color = m_baseColour.ChangeLightness(bottomLightness);
@@ -585,16 +577,9 @@ void wxAuiGenericTabArt::DrawTab(wxDC& dc,
                           tab_width - (text_offset-tab_x) - close_button_width);
 
     // draw tab text
-#if defined( __WXMAC__ )
-    if (page.active)
-    {
-        dc.SetTextForeground(wxSystemSettings::GetColour(wxSYS_COLOUR_CAPTIONTEXT));
-    }
-    else
-    {
-        dc.SetTextForeground(wxSystemSettings::GetColour(wxSYS_COLOUR_INACTIVECAPTIONTEXT));
-    }
-#endif
+    wxColor font_color = wxSystemSettings::GetColour(
+        page.active ? wxSYS_COLOUR_CAPTIONTEXT : wxSYS_COLOUR_INACTIVECAPTIONTEXT);
+    dc.SetTextForeground(font_color);
     dc.DrawText(draw_text,
                 text_offset,
                 drawn_tab_yoff + (drawn_tab_height)/2 - (texty/2) - 1);

Please let me know the results with one of the dark themes.

@hollunder
Copy link
Author

I figure when I have to build wxgtk myself I might as well use a newer version and built 3.1.3. The patch applied cleanly, but wxLua doesn't build against that.
With the gtk2 version it fails pretty much immediately because it can't find some headers, with the gtk2 version during linking.
I'll require a little more time to sort that out. I guess I'll try and patch and build the stable version.

@hollunder
Copy link
Author

Unfortunately the patch fails quite a bit against 3.0.4.

patching file src/common/appbase.cpp
Hunk #1 succeeded at 766 with fuzz 2 (offset 342 lines).
patching file src/aui/tabart.cpp
Hunk #1 FAILED at 267.
Hunk #2 FAILED at 585.
2 out of 2 hunks FAILED -- saving rejects to file src/aui/tabart.cpp.rej

I finally came around to modifying the patch for 3.0.4.. Sorry, I'm not used to working this way and I probably do this in overly complicate ways, hence it took me a bit.

--- a/src/aui/tabart.cpp	2018-03-07 17:55:38.000000000 +0100
+++ b/src/aui/tabart.cpp	2019-12-03 17:07:13.487259312 +0100
@@ -532,6 +532,9 @@
                           tab_width - (text_offset-tab_x) - close_button_width);
 
     // draw tab text
+    wxColor font_color = wxSystemSettings::GetColour(
+	page.active ? wxSYS_COLOUR_CAPTIONTEXT : wxSYS_COLOUR_INACTIVECAPTIONTEXT);
+	dc.SetTextForeground(font_color);
     dc.DrawText(draw_text,
                 text_offset,
                 drawn_tab_yoff + (drawn_tab_height)/2 - (texty/2) - 1);

Attached is a screenshot of that version with the theme Breeze-Dark.
2019-12-03-185106_1366x768_scrot

@pkulchenko
Copy link
Owner

No problem; the screenshot gives me the information I need, thank you!

@pkulchenko
Copy link
Owner

@hollunder, the colors should be fixed in the latest release beta, but the wxwidgets changes are not likely to be ported back to 3.0.4 ;(. Can you try this branch (https://github.com/pkulchenko/ZeroBraneStudio/tree/upgrade-binaries-190d) and let me know if the tab text colors look correct? It should also detect the "dark" theme, so if you are setting the color scheme manually in the config, you can remove that (although you may still need it if your dark scheme is different from the default one.

@pkulchenko pkulchenko added this to the 1.90 milestone Jan 9, 2020
@hollunder
Copy link
Author

Arc-Dark-Solid
2020-01-11_114820

Adwaita-Dark (Adwaita is a relatively common theme, a Gnome default I believe)
2020-01-11_115143

BlackMATE
2020-01-11_115340

Breeze-Dark
2020-01-11_115434

HighContrastInverse (most likely intended as accessibility option)
2020-01-11_115543

Vertex-Dark
2020-01-11_115710

Menta (just to check that light ones still work)
2020-01-11_115749

I hope that selection covers dark themes sufficiently. Looks good on my side.
Thanks for the fix Paul!

@hollunder
Copy link
Author

hollunder commented Jan 11, 2020

Oh, I forgot to remove the styles color scheme setting before making the screenshots. I tested the automatic dark text style briefly with the themes above and it seems to work fine.

@hollunder
Copy link
Author

hollunder commented Jan 11, 2020

As a sidenote, a think I noticed is that the tab background now always has a gradient that it didn't have in 1.80. It sticks out quite a bit with dark themes. I suppose it's there to provide some contrast, but IMHO it's too strong.

2020-01-11_144709

@pkulchenko
Copy link
Owner

@hollunder, thank you for checking with all these examples! Looks good.

I agree on the gradient, but I think it was tweaked in wxwidgets to increase contrast, as you said. It also depends on the background color, so may be too strong when the color is close to 0,0,0.

@hollunder
Copy link
Author

Oh, another thing I noticed just now. There is a very weird halo-like effect on the right side of the editor. It is very noticeable on HighContrastInverse but also visible with the other dark schemes. Hard to imagine that it's intentional. I wonder how I didn't notice this earlier...

@pkulchenko
Copy link
Owner

Do you have a screenshot? I haven't seen this one and can't image what it would be outside of the main frame.

@hollunder
Copy link
Author

Oh I mean this screenshot as the obvious example. It does not extend outside of the program, it's just this weird effect left of the editor window scrollbar. It doesn't seem to appear next to other scroll bars (project, outline, local console).

HighContrastInverse (most likely intended as accessibility option)
2020-01-11_115543

@pkulchenko
Copy link
Owner

Very interesting; not sure what to say or where it's coming from. This is the first time I see this. It does seem to be limited to just that window. It could be some sort of accessibility option, but can't confirm, as I'm not sure what could be triggering it. Do "older" versions of the IDE (like the master branch) show it with the same color scheme?

@hollunder
Copy link
Author

hollunder commented Jan 12, 2020

2020-01-12-104553_3840x1080_scrot
This is 1.80 with the same scheme.
The second image in the first post is also 1.80 with a dark scheme, so there it does not happen.
Pretty much the entire series of dark screenshots above shows this effect, but usually it is more subtle. Only on the BlackMate screenshot it seems to be absent. There the scrollbar itself has a gradient, but that is part of the theme and also appears in other applications.

It gets weirder. At some point when scrolling down the vertical scroll bar appears and that effect disappears.

2020-01-12_110252
2020-01-12_110303
2020-01-12_110616
I don't quite know what to make of this.

@hollunder
Copy link
Author

Another thing I noticed, and this one is I suppose more of a related feature request, is that the tooltip remains white even if a dark theme is detected. It would make sense to have a bright font on dark background for these as well.
2020-01-12-163822_3840x1080_scrot

@pkulchenko
Copy link
Owner

Agree on the tooltip color. This should already be fixed in upgrade-binaries-190d branch.

@hollunder
Copy link
Author

No, this screenshot is from upgrade-binaries-190d.
Btw. can you explain your naming scheme there? What is the difference between all-binaries and upgrade-binaries?

@pkulchenko
Copy link
Owner

all-binaries-190x includes upgrade-binaries-190x plus luasec binaries plus changes in the master branch. It's basically a release candidate, but I'll have all-binaries-190e with all the recent changes shortly.

@pkulchenko
Copy link
Owner

Strange about the tooltip. I do see it colored according to the current color scheme. I'll check on Linux.

@pkulchenko
Copy link
Owner

I don't know what's up, but I suspect some weirdness regarding GTK2/GTK3 theming, but I can't say for certain. Just for future reference, I switched themes using lxappearance, the GTK2 and GTK3 version respectively. I can't rule out that that did something weird either.

@hollunder, thank you very much for the update! Don't hesitate to add a comment to this ticket if you figure out what's going on with the halo or have some additional details.

@pkulchenko
Copy link
Owner

This has been implemented as part of the changes in #999 that have been merged.

@moteus
Copy link
Contributor

moteus commented Apr 12, 2020

I just update ZBS on my Ubunty 18.04 and got the same halo effect. It appears in my case when I set editor.usewrap = false in my config. Also, this effect presents on files without long lines and it dissapera if I type long line. Is there any workaround about this ?

@pkulchenko
Copy link
Owner

@moteus, I don't think so, as it doesn't seem to be controlled from the IDE or Scintilla. I included an answer from Scintilla author to my question in the earlier comment (#1024 (comment)). Does it only happen on the dark background or do you see it with the default color scheme as well?

@moteus
Copy link
Contributor

moteus commented Apr 12, 2020

I see the same on all themes. But it noticeable only on dark one. With the light background it almost not seen.

@ildar
Copy link

ildar commented Apr 16, 2020

Am i getting it right that wx should be patched? Otherwise tabs labels are unreadable?

@pkulchenko
Copy link
Owner

@ildar, no, I applied a patch to wxwidgets that is supposed to fix the contrast on tab labels with a dark color scheme.

Just to confirm, you are using v1.90, right? If you see low contrast on tab labels with the dark mode selected, can you post a screenshot?

@ildar
Copy link

ildar commented Apr 17, 2020 via email

@pkulchenko
Copy link
Owner

I need to upgrade to 3.1.3

This should be sufficient. Unfortunately odd version number is a "dev" version, so may not be available as a system binary. You can definitely take my patch and apply it yourself, but it can be a bit messy, as the changes were in several PRs/updates. See this ticket for the discussion and all related patches: https://trac.wxwidgets.org/ticket/18601

Another problem with 3.1.3 was that there were several serious defects that were fixed shortly after it was released and incorporated into ZBS binaries (the IDE reports 3.1.4 as the version if you check the About screen). So, if you have a choice, I'd recommend building from sources in pkulchenko/wxWidgets instead of wxWidgets/wxWidgets. I don't remember the nature of the defects, but can dig it up if you are interested.

@moteus
Copy link
Contributor

moteus commented Apr 18, 2020

Regarding halo effect. I turn it off using this answer

@pkulchenko
Copy link
Owner

@moteus, great; thank you for the update! I suspect it had something to do with GTK settings, so it's good to have a confirmation. Just in case, here is the solution:

overshoot.top,
overshoot.right,
overshoot.bottom,
overshoot.left {
  background: none;
}

(could also add for undershoot)

undershoot.top,
undershoot.right,
undershoot.bottom,
undershoot.left,

@ildar
Copy link

ildar commented Apr 21, 2020 via email

@pkulchenko
Copy link
Owner

@ildar, just to confirm, these are the libraries your built, right? Does 1 and 2 work with the ones included with the IDE (3 works in both places)? I think 1 was included after 3.1.3 was released and 2 is probably result of a bug in wxwidgets that I mentioned (that also got resolved shortly after the release was made). That's why I suggested building from pkulchenko/wxwidgets master branch, as it includes those changes on top of 3.1.3 (and will report the version as 3.1.4). You can at least check the last commit there, as it will have all those fixes already included.

@ildar
Copy link

ildar commented Apr 22, 2020 via email

@hollunder
Copy link
Author

Thanks, I can confirm this works with the official zbstudio build.
Just to make it more obvious for others how to apply that workaround.

~/.config/gtk-3.0/gtk.css

overshoot.top,
overshoot.right,
overshoot.bottom,
overshoot.left,
undershoot.top,
undershoot.right,
undershoot.bottom,
undershoot.left
{
	background: none;
}

@astrolemonade
Copy link

For some reason the gtk.css workaround doesn't work for me. I still get the overshot effect and I just hate it.
image

@pkulchenko
Copy link
Owner

@cata0309, it's not overshoot; it's the currently selected tabart in the IDE. You may switch to a different one (that doesn't have the gradient) by adding the following to the config: wxaui.wxAuiGenericTabArt = wxaui.wxAuiSimpleTabArt or wxaui.wxAuiGenericTabArt = wxaui.wxAuiDefaultTabArt.

@astrolemonade
Copy link

astrolemonade commented Jun 14, 2020

Could you be more specific ? In what file do I need to edit? I am new to this IDE and I am trying to find my way on mastering it

@astrolemonade
Copy link

got it ! It was /opt/zbstudio/zbstudio/config.lua Now my question is how to make the tabs another color other than white ?

@pkulchenko
Copy link
Owner

got it ! It was /opt/zbstudio/zbstudio/config.lua

That's not the place, as this file will be overwritten upon upgrade. See the Configuration section in the documentation: https://studio.zerobrane.com/doc-configuration#system-wide-configuration

Now my question is how to make the tabs another color other than white ?

These are set based on the system colors, but there is a way to change the default. Adding something like this to the config will change the tab background to Blue and active tab to Green:

local tabart = wxaui.wxAuiSimpleTabArt()
tabart:SetColour(wx.wxColour(0, 0, 255))
tabart:SetActiveColour(wx.wxColour(0, 255, 0))
getmetatable(wxaui.wxAuiGenericTabArt).__call = function() return tabart end

You can also change the fonts according to the documentation here: https://docs.wxwidgets.org/trunk/classwx_aui_tab_art.html

@mimi89999
Copy link

I think you should see some improvement soon. See wxWidgets/wxWidgets#1950 and tell me if you like it.

@astrolemonade
Copy link

What does soon mean in this context ? It could be a week/month/year or maybe more.

@pkulchenko
Copy link
Owner

@mimi89999, the change you are proposing is not relevant to what @cata0309 is discussing, as it doesn't change the background. It's only supposed to change the color of text on tabs (and it doesn't work correctly when the dark/light scheme is switched, as commented in the ticket).

If anything, what's being discussed in https://trac.wxwidgets.org/ticket/18812 may address @cata0309's concern. I'm not sure when it's going to be implemented and integrated.

@mimi89999
Copy link

What does soon mean in this context ? It could be a week/month/year or maybe more.

I hope that the PR will be merged in the following days. https://trac.wxwidgets.org/wiki/Roadmap says we plan to release one last 3.1.4 release in 3.1.x series in May or June 2020

It's only supposed to change the color of text on tabs (and it doesn't work correctly when the dark/light scheme is switched, as commented in the ticket).

No. It changes the color of the background behind tabs as can be clearly seen on the screenshots.

@pkulchenko
Copy link
Owner

I hope that the PR will be merged in the following days. https://trac.wxwidgets.org/wiki/Roadmap says we plan to release one last 3.1.4 release in 3.1.x series in May or June 2020

But this is only for the wxwidgets component; you'll have to recompile the binaries for the IDE itself to get the update.

No. It changes the color of the background behind tabs as can be clearly seen on the screenshots.

It does, but it's not part of this issue that we are discussing; the main issue was that the text was not visible on tabs, which has already been addressed. And this particular change (with the background) was done intentionally, for the reasons I outlined in my comments for https://trac.wxwidgets.org/ticket/18812. I don't mind changing this (as I commented in the ticket), but I don't think your proposal is the way to go, as it has some deficiencies that have already been noted in the ticket.

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

No branches or pull requests

6 participants