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

The curvy underline is muted #4064

Open
Tyriar opened this issue Aug 23, 2022 · 17 comments · May be fixed by #4880
Open

The curvy underline is muted #4064

Tyriar opened this issue Aug 23, 2022 · 17 comments · May be fixed by #4880

Comments

@Tyriar
Copy link
Member

Tyriar commented Aug 23, 2022

VS Code issue microsoft/vscode#158979

The curvy line seems darker than the straight line. Is that expected?
image

@jerch
Copy link
Member

jerch commented Sep 1, 2022

I'd guess thats a visual artefact, like from antialiasing? (it even seems as if the zero crossings are brighter than the slopes...)

No good idea how to fix that - maybe give the curly line more weight, or a tiny bit more luminance? Only guessing here, I have no clue of visual perception tricks...

@Tyriar
Copy link
Member Author

Tyriar commented Sep 1, 2022

I was thinking to review this at a bunch of device pixel ratios on Windows and Mac and tweak it to make it more consistent looking, probably just bumping the line width up and maybe the curve too. There is some rounding logic going on with the other lines also to make them solid pixel lines without AA if possible, this could be adding to them looking a little different.

@jerch
Copy link
Member

jerch commented Sep 1, 2022

probably just bumping the line width up and maybe the curve too.

Maybe making the curve steeper in the zero crossing will already do? At least thats the main difference I see to my old attempt here: https://user-images.githubusercontent.com/6193135/76248036-d50c3f00-6240-11ea-855b-37e8aec72014.png

Oh well that also was 3 px high, looking at your example above it might still work with 1px more for the curve height.

@Tyriar
Copy link
Member Author

Tyriar commented Sep 2, 2022

That should make the line more solid as it's straight, but it's not exactly curvy. Will see how it looks after some tweaks vs the zig-zag/squiggly approach.

@Tyriar
Copy link
Member Author

Tyriar commented Sep 2, 2022

Here's monaco's:

image
image

I think it uses a repeating underline image, the underline in the webgl renderer is included in the glyph's texture so it currently must be repeatable by the width of a cell:

image

@Tyriar
Copy link
Member Author

Tyriar commented Sep 2, 2022

Here's Chromium's:

image

This was the one that looks terrible because it appears to be cached with the glyph and doesn't repeat

@Tyriar
Copy link
Member Author

Tyriar commented Sep 2, 2022

Dotted 🤢

image

@jerch
Copy link
Member

jerch commented Sep 2, 2022

Ah monaco's curvy line seems to use the same progression in px as I did. Would it be possible to make a "wave painting function" from that without restarting it for every cell (thus dependent from screen position progression) to avoid cell stitching artefacts? (If thats possible at all, cause it might need to draw the wavy thing separately in a whole and subtract glyph overlappings afterwards.)

Chromium clearly wins the stitching prize here...

@Tyriar
Copy link
Member Author

Tyriar commented Sep 2, 2022

Would it be possible to make a "wave painting function" from that without restarting it for every cell (thus dependent from screen position progression) to avoid cell stitching artefacts?

Two ways this could happen:

  1. Separate the underline rendering completely, resulting in essentially another glyph renderer. This would be too complicated/overkill just to make curvy overlines look good.
  2. When curvy underline is used, create a separate glyph for every x cell coordinate. This would look great, but add more complexity to how glyph caching works (passing through the x), and cost more memory which probably isn't a big deal.

My gut feeling is these are both overthinking it and we should just do what Chromium does but do it right; similar to what we have now but more polished.

@jerch
Copy link
Member

jerch commented Sep 2, 2022

I agree, both seem overkill for what is to be gained here.

@tisilent
Copy link
Contributor

I want to give it a try.

@Tyriar
Copy link
Member Author

Tyriar commented Aug 11, 2023

@tisilent sure, I expect this one to be a bit difficult though as we need to make it look good at so many zoom levels/font sizes

@tisilent
Copy link
Contributor

tisilent commented Nov 9, 2023

66
Would this look better. 🤪

@jerch
Copy link
Member

jerch commented Nov 9, 2023

Right side is def. better than left imho - but, can you turn every second curve upside-down, so it becomes more like a sine wave than this spikey pattern?

@tisilent
Copy link
Contributor

tisilent commented Nov 10, 2023

12
@jerch hi. Can you feel some curvature?

@Tyriar
Copy link
Member Author

Tyriar commented Nov 10, 2023

@tisilent definitely look better, that's a lot like the zig zag-style curly style of VS Code's problems. Does it look ok if you stretch the wave to be 1-2 pixels wider to smooth the curve a little more?

I notice something going wrong towards the end:

image

@tisilent
Copy link
Contributor

@Tyriar Stretching 1-2px will increase the variants, I'll give it a try.

@tisilent tisilent linked a pull request Nov 11, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants