Limit capture by samples per point, not by pixel width - #27
Merged
Merged
Conversation
The capture width is capped at a fixed 2400 pixels, so a larger built in display gets a softer fold than a smaller one: the same pixels are spread over more points. A 1728 point display resolves 1.39 samples per point where a 1512 point display resolves 1.59, and the difference is visible in the fold even with the same chip behind it. Cap the ratio at 1.6 instead, with a 2800 pixel guard rail above it. Sharpness is then the same on every built in display. The smaller display is unchanged in practice, because 1.6 is where its old limit already landed, and a 1728 point display goes from 2400 to 2764 pixels wide. Everything downstream is already proportional, so nothing else needs to move: the blur sigmas scale with the small texture width, and the shader works entirely in normalized coordinates. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@reesoousa is attempting to deploy a commit to the MagicAPI Team on Vercel. A member of the Team first needs to authorize it. |
|
Warning Review limit reachedNext included review available in 41 minutes. View limit detailsLimit details: You’ve used the included review currently available. This review ran on the open-source allowance, not this organization's plan, because the pull request author doesn't have an assigned seat. Waiting won't change this — ask an organization admin to assign them a seat, or add seats in Billing if every seat is already assigned, then retry. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The capture width is capped at a fixed number of pixels:
A fixed pixel budget spread over a larger display means fewer samples per point, so the fold is softer on a bigger MacBook than on a smaller one with the same chip behind it:
This turned up from a 16 inch owner reporting the effect looked worse than on a 14 inch, which is what sent me to this line.
Change
Cap the ratio rather than the absolute width, so sharpness is the same on every built in display, and keep an absolute guard rail for any future larger panel.
1.6 is deliberate: it is where the old limit already landed on a 1512 point display, so the smaller machine is unchanged in practice and there is no regression to trade for the fix. A 1728 point display goes from 2400 to 2764 pixels wide, about a third more fragments through the blur chain, on hardware in the same family as the smaller machine that already handles it.
Why nothing else moves
Everything downstream of this line is already proportional, so the one line is the whole fix:
$0 * smallWidth / 786), so they scale with the capture and stay the same fraction of the screenFold.metalworks entirely in normalized coordinates; its constants are proportions, not pixelsTesting
Built with
make buildon an M4 MacBook Pro, macOS 26.6.2, where the change is a no op by design (1.587 to 1.600). The larger display case is reported rather than measured by me: I do not have a 16 inch to hand, so I am relying on the arithmetic above plus a user report, and I would rather say so than imply I verified it on both panels.I could not run
npm run checklocally: theswift-formatshipped with the Command Line Tools cannot read this repo's configuration schema, so CI lint is the authority on formatting here.