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

Fix tab control flickering in dark theme #9049

Merged
merged 5 commits into from Apr 13, 2021
Merged

Fix tab control flickering in dark theme #9049

merged 5 commits into from Apr 13, 2021

Conversation

NikolayXHD
Copy link
Member

Fixes

tab control flickering introduced by #8815

screenshot

Proposed change

as discussed #8841 (comment)

  1. Revert Fix tab control border in high dpi #8815
  2. Fix high DPI issues in the code which renders TabControl manually.

Screenshots

image

Test methodology

manually

Test environment(s)

  • Windows 10
  • UI scale 150%

✒️ I contribute this code under The Developer Certificate of Origin.

@ghost ghost assigned NikolayXHD Apr 4, 2021
var outerRect = GetOuterTabRect(index);
_graphics.FillRectangle(GetBackgroundBrush(index), outerRect);

var points = new List<Point>(4);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stackalloc Point[4]?

Copy link
Member

@gerhardol gerhardol Apr 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

related, nit pick (generic)
We have used explicit types and C#9 new()
https://docs.microsoft.com/en-us/dotnet/csharp/whats-new/csharp-9#fit-and-finish-features

            List<Point> points = new(4);

Copy link
Member Author

@NikolayXHD NikolayXHD Apr 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stackalloc Point[4]

can't do, the actual size is 2-4 depending on index

List<Point> points = new(4);

ok

@RussKie RussKie requested a review from a user April 5, 2021 13:36
@RussKie
Copy link
Member

RussKie commented Apr 5, 2021

/cc: @mdonatas

@RussKie RussKie added this to the 3.5 milestone Apr 5, 2021
@mdonatas
Copy link
Contributor

mdonatas commented Apr 5, 2021

The flickering is gone 👍 but something felt wrong so I tried an older build.

This PR:
image

Some build from 3.3 days I had at hand:
image

In 3.3 days the top border would get the color of the active tab.. I think that's the reason something seemed of with tabs in this PR.
Alternatively in 3.3 there might've been no borders.. since their color is same as surrounding background it's hard to tell.

Copy link
Member

@gerhardol gerhardol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1
Run fine with 150%, 200% (no restart though). I do not really see the problem also without this change though.
I normally only use 100% myself, so I do not encounter these issues in normal usage.
I tried to look at border values etc for a few situations, looked fine to me.

@NikolayXHD
Copy link
Member Author

NikolayXHD commented Apr 5, 2021

The flickering is gone but something felt wrong so I tried an older build.

This PR:
image

Some build from 3.3 days I had at hand:
image

changed hot tracked tab color in review commit

@NikolayXHD NikolayXHD requested a review from RussKie April 5, 2021 22:25
@mdonatas
Copy link
Contributor

mdonatas commented Apr 6, 2021

bottom-border-color

Watch the bottom border of the tab button, it changes color on/after mouse over so the buttons that had been mouse-overed have a different bottom border color than those that haven't.

image
See that the one that hasn't been hovered has a white bottom border.

@NikolayXHD
Copy link
Member Author

NikolayXHD commented Apr 6, 2021

bottom-border-color

Watch the bottom border of the tab button, it changes color on/after mouse over so the buttons that had been mouse-overed have a different bottom border color than those that haven't.

image
See that the one that hasn't been hovered has a white bottom border.

@mdonatas Looks to me the visual artifact happens with "Use system default visual style" flag enabled.
I can tell it from the shape of scrollbar arrow, which makes appearance in the gif for a brief moment.
Correct me if I am wrong about that.

With that flag it's the system drawing the tab control, our (my) code has nothing to do with this.

if (ThemeSettings.UseSystemVisualStyle)

Also, I cannot reproduce it. Tried with 150%, 175% DPI, there was no gray 1px line below tabs. Probably it requires 200% DPI, which Windows display settings does not allow me to choose.

175% DPI, "Use system default visual style" flag off. No grey 1x line below tabs.

image

@mdonatas
Copy link
Contributor

mdonatas commented Apr 7, 2021

Edit: In both master and this PR builds I have "Use system default visual style" disabled.
Edit2: I have no idea why that chevron on a scrollbar looked like system painted it but I can repro all the same behavior in that gif with custom painted chevron (that is with this setting turned off).
image

In any case, I have three builds
Older 3.3 (the arrow is different here but there was no "Use system default visual style" setting back then)
image

Flickery master
image

This PR (notice scrollbar arrow/chevron)
image

And only in this PR is there an underline in places where it should not be... so maybe it's not your code but it's because of the changes made in this PR.

One other thing, Console tab is the only that doesn't change color when activated... weird
image

@mdonatas
Copy link
Contributor

mdonatas commented Apr 7, 2021

Hmm... ok, so there is something related to this Console tab as it the only one which gets different active color and paints the area beneath it with it. If I don't touch it then there are no artifacts.

Also same here with 175% DPI, couldn't reproduce this underline.

@mdonatas
Copy link
Contributor

mdonatas commented Apr 7, 2021

Uuuu.... thanks for showing me this Use system default visual style setting! GE looks better with it!
Look at the tabs! Console tab has correct color and no underline.
image

A blue hover color is back!

And this area in the toolbars looks way better!
This looked like a mess
image

This looks a little bit less like a mess :D
image

Update: To be fair, some things do look better with this setting turned off i.e. hover color of toolbar buttons and menu items.

Would it be wrong to argue that unless a user wants black theme color we should have this setting on by default?

@NikolayXHD
Copy link
Member Author

Would it be wrong to argue that unless a user wants black theme color we should have this setting on by default?

Right, and it matches what current behavior does.

When user selects "default" theme from the list, "Use system defined visual style" checkbox is disabled, controls are rendered by operating system. If OS is dark, GE is dark and vice versa.

With non-default theme "Use system defined visual style" is unchecked and becomes editable. This is because we need to render colors different from current OS theme.

@mdonatas
Copy link
Contributor

mdonatas commented Apr 8, 2021

Right, and it matches what current behavior does.

Oh, sorry, I didn't notice that.

Can you reproduce Console tab being of a different color when active than any other tab? I think that might be a clue for why there is an underline when it's activated.

@gerhardol
Copy link
Member

I suggest this is merged as is both in master and 3.5 and further discussions are handled separately.
The remaining visual glitches are minor (if I have to look this hard for them they I personally do not care at all, but others are welcome to have other opinions).
Open a new issue instead.

@NikolayXHD
Copy link
Member Author

I plan to look at this tomorrow. If I can't fix it tomorrow, i'll do as @gerhardol suggests, merge this and separate the remnant to another issue.

@RussKie
Copy link
Member

RussKie commented Apr 10, 2021 via email

@NikolayXHD
Copy link
Member Author

Can you reproduce Console tab being of a different color when active than any other tab? I think that might be a clue for why there is an underline when it's activated.

@mdonatas what you said turns right, it was enough to make console tab render the same background color, to get rid of the artifact.

Copy link
Member

@RussKie RussKie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you

Squash and merge

@NikolayXHD NikolayXHD merged commit 1783b48 into gitextensions:master Apr 13, 2021
@ghost ghost modified the milestones: 3.5, 3.6 Apr 13, 2021
@mdonatas
Copy link
Contributor

Great! Thanks @NikolayXHD

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

4 participants