online-go / goban

A JavaScript library for exploring and playing the game of Go
Apache License 2.0
47 stars 39 forks source link

GobanCore: round square size to device pixels instead of arbitrary px units #153

Closed pdg137 closed 4 months ago

pdg137 commented 8 months ago

This uses the device resolution to round "squareSize" to the nearest units in device pixels. For example, on my 14" laptop screen, with a devicePixelRatio of 1.25, 1 pixel is actually 0.8px. So I get square sizes of 60, 60.8, 61.6, etc.

I have not looked into what the size is used for, so I don't really know if it's okay for it not to be an integer. But here is an example screenshot on Pixel 7 showing that it at least works in some sense:

Screenshot_20240217-183543

Here's what it looks like before the change, maybe there are slightly fewer artifacts but that could be an accident:

image

benjaminpjones commented 8 months ago

I tried to see if there's a noticeable difference with non-integer square size, and I am noticing some more artifacting on non-integer sizes. Here are some key numbers/display info for reproducibility:

On Beta

Screenshot 2024-02-17 at 8 21 01 PM

Pixel-perfect lines (2px, uniform color):

Screenshot 2024-02-17 at 8 21 47 PM

On This PR

Breaks at the line mid points:

Screenshot 2024-02-17 at 8 26 24 PM

Aliasing of the lines (lines are about 3 pixels wide with the outer edges blended with the board color)

Screenshot 2024-02-17 at 8 27 17 PM
pdg137 commented 8 months ago

Maybe we can fix these issues by rounding appropriately before drawing. I tried a little on the latest commit and got it to look kind of nice in the Chrome inspector:

image

And on a Pixel 6 with pixel ratio 2.625:

image

We can probably do better by making the line thickness be an integer number of device pixels and also extending the lines a bit where they don't join up very well in the middle of a square.

pdg137 commented 8 months ago

Related:

https://forums.online-go.com/t/request-customize-line-width/50810

benjaminpjones commented 8 months ago

Took a look at the recent commit, and the lines look a lot crisper!

Noticing a couple more things that seem to happen with non-integer square sizes:

Star points

Circles are off center by 1px. This happens consistently, but is more noticeable on smaller screens. For example, iPhone SE (small screen, but dpr=2):

Centered (beta):

Screenshot 2024-02-18 at 10 24 51 PM

Uncentered (this PR):

Screenshot 2024-02-18 at 10 27 12 PM

Corners

Corners not aligned. This also happens consistently and is more noticeable IMO

Aligned:

Screenshot 2024-02-18 at 11 01 40 PM

Misaligned (happens on non-integer square sizes):

Screenshot 2024-02-18 at 10 14 29 PM Screenshot 2024-02-18 at 10 14 36 PM
pdg137 commented 8 months ago

Right, I didn't try to do anything with the star points or ends, so far just messed with the line positions.

If we want this to be better, I think someone needs to take a careful look over the whole drawing code and figure out what to do with all the coordinates in a principled way. One question is whether the coordinates even have to be in px? It might be more straightforward if we could draw everything using actual device pixels - but taking px into consideration as well.

For example, currently the line with is 1px. Maybe instead it should be 1px rounded up to the nearest integer number of device pixels.

anoek commented 8 months ago

Something else to throw out there, I've had it in the back of my mind to explore creating a SVG rendering backend instead of using canvas rendering. I think at this point the canvas based rendering isn't so necessary these days and there are some notable downsides and problems we run into, if an SVG based renderer is performant enough it'd likely solve all you're trying to change here.

benjaminpjones commented 8 months ago

I think SVG could be performant for sure. However I don't think it is straightforward to do the pixel-perfect calculations* that goban currently does. Is this something we can forget about, given that screens are much higher res than they use to be?

I think it would make for much simpler code and layout issues would no longer be an issue, but I did want to check because Goban code goes to great lengths for pixel precision.


*namely, calculations that attempt to solve these constraints:

pdg137 commented 8 months ago

There are still a lot of people on 1px = 1 pixel screens (I'm using one as I type this). At higher resolutions it might be okay to be a little blurry (like it is already) but it would be better if it weren't! And some of the tests I did here made it a lot worse, so it doesn't sound great to have less control.

anoek commented 4 months ago

Feel free to open this back up if you want to pick it back up again. It might be moot with the new SVG renderer, although the current plan is to maintain both renderers at least while we discover what browser kinks and problems we run into when we deploy the SVG renderer to everyone as the default, so it might still be relevant if we decide to keep the canvas renderer as an option or the default.