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

feat: Physical pixel snapping #837

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

Tropix126
Copy link
Contributor

This is a rewrite of #245 that's up to date. Same deal - it just rounds all elements to render on physical pixel boundaries rather than relying subpixel antialiasing which results in blurry borders or incorrect rendering in the case of #836.

@Tropix126 Tropix126 changed the title feat: round to physical pixel boundaries while rendering feat: Physical pixel snapping Sep 1, 2024
Copy link

codecov bot commented Sep 1, 2024

Codecov Report

Attention: Patch coverage is 40.38462% with 31 lines in your changes missing coverage. Please review.

Project coverage is 75.92%. Comparing base (13f67c5) to head (b1b9fd1).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
crates/devtools/src/tabs/style.rs 0.00% 8 Missing ⚠️
crates/core/src/elements/rect.rs 16.66% 5 Missing ⚠️
crates/core/src/elements/svg.rs 0.00% 5 Missing ⚠️
crates/state/src/style.rs 77.27% 5 Missing ⚠️
crates/core/src/elements/image.rs 0.00% 4 Missing ⚠️
crates/core/src/node.rs 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #837      +/-   ##
==========================================
- Coverage   76.04%   75.92%   -0.12%     
==========================================
  Files         205      205              
  Lines       23235    23290      +55     
==========================================
+ Hits        17670    17684      +14     
- Misses       5565     5606      +41     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@marc2332 marc2332 added the enhancement 🔥 New feature or request label Sep 1, 2024
Copy link
Owner

@marc2332 marc2332 left a comment

Choose a reason for hiding this comment

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

The switch animation looks weird now, and I don't think disabling the rounding just for the switch is a good solution either.

@Tropix126
Copy link
Contributor Author

The switch animation looks weird now, and I don't think disabling the rounding just for the switch is a good solution either.

Any suggestions on what I should do here instead? I'll look into the switch animation.

@Tropix126
Copy link
Contributor Author

Tropix126 commented Sep 14, 2024

I've fixed the alignment of the switch by disabling rounding. I'm not sure what's causing the animation to look weird (it seems fine to me although I might be missing something). I'm open to suggestions for alternatives other than disabling rounding on the switch, but any solution other than disabling it is probably going to inevitably mess up on some scale factors.

Before (offcenter, pixel snapped) After (centered via antialiasing)
image image image image

@marc2332
Copy link
Owner

Try resizing the window vertically.

Eventually you will see how the inner circle gets missaligned

image

When this happens and you run the animation, it will appear as:

Recording.2024-09-14.184639.mp4

@Tropix126
Copy link
Contributor Author

Hmm... Okay try now, I think it should be fixed on switch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 🔥 New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants