Skip to content

Fix y-axis offset in feSpotLight transform_light_source#1052

Open
SAY-5 wants to merge 1 commit into
linebender:mainfrom
SAY-5:fix-spotlight-region-y
Open

Fix y-axis offset in feSpotLight transform_light_source#1052
SAY-5 wants to merge 1 commit into
linebender:mainfrom
SAY-5:fix-spotlight-region-y

Conversation

@SAY-5

@SAY-5 SAY-5 commented May 21, 2026

Copy link
Copy Markdown
Contributor

transform_light_source was subtracting region.x() from the y-component of both light.y and light.points_at_y in the SpotLight branch (crates/resvg/src/filter/mod.rs:1079, 1085). Whenever the filter region origin has x != y, the spotlight is offset on the y axis by region.x() - region.y() pixels, producing mis-rendered specular/diffuse lighting.

The adjacent PointLight branch already uses region.y() for the y subtraction (line 1070), so the SpotLight branch is a copy-paste bug. This change brings SpotLight in line with PointLight and the SVG filter spec.

Existing filter tests (all 249) still pass; they happen to use filter regions anchored at the origin so the bug was not surfaced in the snapshot suite.

`transform_light_source` was subtracting `region.x()` from the y-component
of both the light source position and its `points_at` target in the
`SpotLight` branch. The result is that whenever the filter region origin
has `x != y`, the spotlight is offset by `region.x() - region.y()` pixels
on the y axis, producing mis-rendered lighting. The adjacent `PointLight`
branch uses `region.y()` correctly, so this is a copy-paste bug introduced
when the SpotLight branch was derived from PointLight.
@rbtz-openai

Copy link
Copy Markdown

nit: maybe we can adjust tests a bit so that they spot this bug and provide regression tests if it re-appears?

@SAY-5

SAY-5 commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

Good call. I'll add a render test with a filter region where x != y (the bug is invisible at the origin), so the snapshot would shift if the SpotLight y-offset regresses.

@SAY-5

SAY-5 commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

I tried to add a render test for this and hit a snag worth flagging: with an axis-aligned filter region the wrong y-offset shifts the spotlight apex, but feDiffuseLighting's per-pixel normal term dominates enough that a fresh reference PNG comes out byte-identical, so the test wouldn't actually fail without the fix. The defect only changes output once a non-identity transform makes region.x() and region.y() diverge in a way the cone edge sees (the existing complex-transform fixture is the closest case). I'd rather not commit a test that passes both ways. If you'd prefer, I can add a transform-based feSpotLight fixture whose reference visibly changes, but wanted to confirm that's the direction before regenerating a snapshot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants