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

Proper Pane Grid spacing #361

Merged
merged 3 commits into from May 28, 2020
Merged

Conversation

clarkmoody
Copy link
Contributor

On low-DPI screens, the rounding order of operations made it impossible to produce an odd-pixel spacing. Specifying 1, for instance, produced zero space between panes.

This approach subtracts half the spacing from the first pane prior to rounding and uses the whole spacing for the second pane size and coordinate.

We're no longer pre-computing the halved_spacing and passing that around. Rather Axis::split operates on the full spacing.

On low-DPI screens, the rounding order of operations made it impossible
to produce an odd-pixel spacing. Specifying 1, for instance, produced
zero space between panes.

This approach subtracts half the spacing from the first pane prior to
rounding and uses the whole spacing for the second pane size and
coordinate.
Copy link
Member

@hecrj hecrj left a comment

Choose a reason for hiding this comment

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

Thanks!

On low-DPI screens, the rounding order of operations made it impossible to produce an odd-pixel spacing. Specifying 1, for instance, produced zero space between panes.

I believe the spacing was there but, as the halved_spacing was subtracted after rounding, the resulting rectangles were not aligned to the logical pixel grid. This can happen as part of layouting, so it shouldn't be a problem!

The changes here align the bounds to the logical grid after subtracting the spacing. This is more desirable because it's easier for the renderer to target physical pixels when dealing with an integral scale factor. However, it won't fix the issue when using a non-integral one.

Thus, the real underlying problem remains unsolved: targetting physical pixels when the rendering primitives do not align perfectly with the pixel grid (clamping?). The fix should probably happen in the quad rendering pipeline in iced_wgpu. I will try to play with it.

@clarkmoody
Copy link
Contributor Author

Thus, the real underlying problem remains unsolved: targetting physical pixels when the rendering primitives do not align perfectly with the pixel grid (clamping?). The fix should probably happen in the quad rendering pipeline in iced_wgpu. I will try to play with it.

Might be a configuration option. SVG has the shape-rendering attribute, which may take the value crispEdges. There we find:

To achieve crisp edges, the user agent might turn off anti-aliasing for all lines and curves or possibly just for straight lines which are close to vertical or horizontal. Also, the user agent might adjust line positions and line widths to align edges with device pixels.

@hecrj
Copy link
Member

hecrj commented May 26, 2020

@clarkmoody I believe quads should always be pixel-aligned, independently of the anti-aliasing strategy. They are GUI rendering primitives that should always be crisp.

The Mesh primitive, together with the antialiasing flag in Settings, can be leveraged to render geometry with subpixel precision.

I have opened #362 which should fix the main issue here. I still believe we should merge this, though!

native/src/widget/pane_grid/axis.rs Outdated Show resolved Hide resolved
@hecrj hecrj added the improvement An internal improvement label May 26, 2020
@hecrj hecrj added this to the 0.2.0 milestone May 26, 2020
Copy link
Member

@hecrj hecrj left a comment

Choose a reason for hiding this comment

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

Awesome! Thank you 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement An internal improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants