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

Vivid curly #4880

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from
Draft

Vivid curly #4880

wants to merge 12 commits into from

Conversation

tisilent
Copy link
Contributor

fix #4064

@tisilent
Copy link
Contributor Author

tisilent commented Nov 11, 2023

before:
before

after:
after

To be resolved....
When fontSize 40, before:
40b

When fontSize 40, after:
40a

Using more variants can solve this problem, I'm wondering if there's any other way.........

@tisilent tisilent marked this pull request as draft November 11, 2023 15:28
@tisilent
Copy link
Contributor Author

@Tyriar hi.

fontSize 25

style 1:
zoom1

style 2:
zoom2

Style 2 can be further beautified.
But when the cell width is singular, there is still no way to solve it.

@Tyriar
Copy link
Member

Tyriar commented Nov 13, 2023

@tisilent these two have the ideal shape:

image
image

They just need some anti-aliasing

@tisilent
Copy link
Contributor Author

tisilent commented Nov 13, 2023

They just need some anti-aliasing

anti-aliasing
😯

next: cellWidth 19
cell19
!!!

@jerch
Copy link
Member

jerch commented Nov 13, 2023

@tisilent Is that still a tiling algo approach? Your last image kinda implies that. I am not sure, if a tiling algo will ever satisfy glitch-free drawing for a curly line at different font sizes - you'd basically need to find several tiling pattern for different zoom levels / font-sizes in the end.

(I may repeat myself here...) I suggest to give up the cell tiling approach for the curve drawing. Instead it might be much easier to find several good looking curves at different font-sizes, then draw those to the full viewport width into a cache, and during rendering clip draw parts of them under cells with curly underlines. Should to the trick.
This idea gives up a certain curve shape at the start/end of a cell, which should give you more freedom to draw a nice curly line w'o stitching/aliasing artifacts. The important trick is to take the part of the whole cached line directly under the cells in question, which always gives you a perfect continuation of the curly progression.

Our eye/perception is more aware of tiny shape breaks on repeating pattern, than it would get bothered by curly shapes not perfectly in line with cell/glyph borders.

@tisilent
Copy link
Contributor Author

tisilent commented Nov 14, 2023

@jerch This is an attempt made with the existing structure unchanged, It seems that it cannot be perfectly solved...

you'd basically need to find several tiling pattern for different zoom levels / font-sizes in the end.
Yes, it's quite troublesome. It's not a good practice.

(I may repeat myself here...) I suggest to give up the cell tiling approach for the curve drawing. Instead it might be much easier to find several good looking curves at different font-sizes, then draw those to the full viewport width into a cache, and during rendering clip draw parts of them under cells with curly underlines. Should to the trick.
It should be possible to combine it and achieve better performance. ☺️

@tisilent
Copy link
Contributor Author

// Reset overrides variantOffset
this.result.ext &= ~ExtFlags.VARIANT_OFFSET;
this.result.ext |= ($variantOffset << 29) & ExtFlags.VARIANT_OFFSET;

An extra 3 bits will solve the problem...

@jerch
Copy link
Member

jerch commented Nov 14, 2023

It should be possible to combine it and achieve better performance. ☺️

Yes, there are perf implications from my idea above - it would need a quite different row drawing setup like an additional row composition step, e.g. first mask under/overline parts into the empty row, then clip glyphs onto that. This is most likely slower than doing all at cell level at once (the tiling approach suits the kernel idea in opengl much better). Whether thats unbearable slower I cannot tell though.
It might be possible to stick to the cell tiling approach also for the over/underlines, if you find a function fully describing such a full viewport width (curly) line by also depending on the cell position in the row, e.g. curly(fontSize, CellXPos) --> ... and allow some "overdrawing" to the right and left to level out stitching breaks.

@Tyriar
Copy link
Member

Tyriar commented Nov 14, 2023

I find the variant tile idea is quite elegant as everything is done initially when drawing the texture and assigned a new identifier, no additional draw steps are needed. This is why the current curly pattern repeats every cell.

To make this nicer with variants we could just keep the pattern repeating every cell (ie. variant 0) and then only when needed to make the underline more natural looking can we add additional variant(s). So based on the width of the cell determine the maximum number of variants to be used which will define the shape of the curl. This is just improving the current situation by giving us more possible shapes to play with.

@tisilent
Copy link
Contributor Author

@jerch Variant starts from here #4349

@tisilent
Copy link
Contributor Author

first

😋

Code not processed I'll take care of it these days.
It also expands the memory footprint and requires optimization.
Testing, etc.....

@tisilent tisilent marked this pull request as ready for review November 18, 2023 11:00
@tisilent
Copy link
Contributor Author

@Tyriar Using underlineOfferset,I want to migrate from ext related to underline,But there were some significant changes.
Also changed the Map to 5 keys.Look at those that need to be adjusted.

@tisilent
Copy link
Contributor Author

fontSize 36
36-1
36-2

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.

The curvy underline is muted
3 participants