fix: size the DVD logo from the short axis, so it stops being huge on the wall (#117) - #252
Merged
Conversation
… the wall (#117) Owner: "the logo is too big". Measured at 6000x1200 before this change, it covered 871 x 238 px -- close to a metre across on the real wall. THE AXIS WAS WRONG, NOT JUST THE NUMBER The logo was sized as a fraction of canvas WIDTH, with height derived to preserve its pixel aspect: logoW = 0.10..0.15 of width logoH = logoW / aspect * (width / height) The derivation is correct. Anchoring to the width is not. On a 5:1 canvas 12% of 6000px is 720px wide, and the derived height follows it up until the logo spans a fifth of the frame's height. Reversed: height is now chosen from the short axis and width derived. That is what every other saver in this folder does, for the same reason -- pointScale and particleSide in gl-base.js, CELL_PX in ascii-donut.js, the cell sizing in truchet.js. A size in angular terms has to come from the dimension that does not stretch. MEASURED, BOTH AXES 6000x1200 871 x 238 px (19.8% of height) -> 519 x 143 px (11.9%) 1920x1080 280 x 76 px ( 7.0% of height) -> 466 x 126 px (11.7%) The wall figure is what was asked for: 40% smaller linearly, 63% less ink. The 16:9 figure goes the other way and that is the same bug, not a regression. The old sizing made apparent size depend on the canvas SHAPE -- 19.8% of height on 5:1 against 7.0% on 16:9, a 2.8x swing from aspect alone. It was too big on the wall and a speck on a laptop for one reason. After the change both read ~11.8%, which is the whole point. This has been fixed once before -- "DVD logo screensaver drawn smaller", v2.6.2. It came back because the size stayed anchored to the width, so widening the canvas re-grew it. Anchored to the short axis, it cannot. shadercheck ok=150 fail=0. DVD Logo's measured density rose from 0.0026 to 0.0065 rather than falling, because the harness canvas is near-16:9 where this change makes the logo larger; the check only fails on drops and 2.5x is inside the staleness threshold, so no baseline change. The sub-margin set is unchanged, still Plasma alone. Addresses the size complaint only. The original items on #117 -- motion blur, bloom, an actual corner hit -- are untouched.
10 tasks
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.
Refs #117 — addresses the size complaint folded into that issue. Its original items (motion
blur, bloom, an actual corner hit) are untouched, so it stays open.
The complaint
Measured at 6000×1200 before this change: 871 × 238 px, close to a metre across on the real
wall.
The axis was wrong, not just the number
The logo was sized as a fraction of canvas width, with height derived to preserve its pixel
aspect:
The derivation is correct. Anchoring to the width is not. On a 5:1 canvas, 12% of 6000px is
720px wide — and the derived height follows it up until the logo spans a fifth of the frame's
height.
Reversed: height is now chosen from the short axis and width derived. That is what every
other saver in this folder does, for the same reason —
pointScaleandparticleSideingl-base.js,CELL_PXinascii-donut.js, the cell sizing intruchet.js. A size in angularterms has to come from the dimension that does not stretch.
Measured, on both aspects
The wall figure is what was asked for: 40% smaller linearly, 63% less ink.
The 16:9 figure goes the other way, and that is the same bug rather than a regression. The old
sizing made apparent size depend on the canvas shape — 19.8% of height at 5:1 against 7.0% at
16:9, a 2.8× swing from aspect alone. It was too big on the wall and a speck on a laptop for
one single reason. After the change both read ~11.8%, which is the entire point.
If you would rather 16:9 keep its old smaller logo, that needs a second size anchored to
something else and I would argue against it — it reintroduces exactly the shape-dependence this
removes.
It has been fixed once before
"DVD logo screensaver drawn smaller" — v2.6.2. It came back because the size stayed anchored to
the width, so widening the canvas re-grew it. Anchored to the short axis, it cannot.
Verification
main(926c4ef).npm run lintclean,npm test— 416 passed.npm run shadercheck—ok=150 fail=0, no baseline change. DVD Logo's measured densityrose from 0.0026 to 0.0065 rather than falling, because the harness canvas is near-16:9 where
this change makes the logo larger. The check only fails on drops, and 2.5× is inside the
staleness threshold added in fix: report stale structure baselines, and correct three of them (#227) #234. The sub-margin set is unchanged — still Plasma alone, as its
test asserts.
Not verified
previous linear size, which is what "too big" implied, but the target was my choice —
LOGO_H_RANGE = [0.09, 0.12]is one constant to change if it should be smaller still.is no reason to expect a change, but I have only looked at stills.