From 7cff41efbd7c22ab1669a111116602f0339b8ecc Mon Sep 17 00:00:00 2001 From: samyfodil Date: Sun, 2 Aug 2026 19:03:54 -0500 Subject: [PATCH] fix(render): keep an SVG inside the bounds it was given MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RenderSVG on the scene canvas positions a document with a transform built from the bounds it is handed, but never constrains it to them. Anything the document draws past its own viewBox therefore lands on whatever surrounds the icon. That is not an unusual document. An SVG may draw anywhere, and every renderer that follows the spec clips to the viewport; icon sets rely on it, since a stroke centred on the viewBox edge puts half its width outside. So the common case is a bleed of a pixel or two, invisible wherever another widget paints afterwards and covers it. It becomes visible on the last icon in a row, or one against the edge of a window, where nothing paints over the spill: in a terminal emulator on this toolkit a 16x16 gear in the tab bar's trailing cluster painted a block from its own origin to the corner of the window, over the tab bar. This arrived with the switch to vector re-emission (#200). The previous path rasterised the document into a bitmap sized from those same bounds, so the bounds were enforced by construction — nothing could escape a raster that size. Emitting geometry into the scene removed that, and nothing took its place. Two changes, both making RenderSVG behave like its neighbours: - Push a clip of the given bounds around the emission, so the document is bounded the way the raster used to bound it. - Honour the current clip. Every other draw method on this canvas returns early when isVisible is false; RenderSVG was the one that did not, so an icon with no pixels on screen still emitted its whole document. Tests cover both, using a document that deliberately draws four times its viewBox — the visible version of the stroke case. Scene.Bounds() is the union of the shapes and does not narrow for a clip, so the test checks the emitted commands instead. --- internal/render/scene_canvas.go | 17 +++++ .../render/scene_canvas_svg_bounds_test.go | 74 +++++++++++++++++++ 2 files changed, 91 insertions(+) create mode 100644 internal/render/scene_canvas_svg_bounds_test.go diff --git a/internal/render/scene_canvas.go b/internal/render/scene_canvas.go index 5a1c004..28517f3 100644 --- a/internal/render/scene_canvas.go +++ b/internal/render/scene_canvas.go @@ -766,12 +766,29 @@ func (c *SceneCanvas) RenderSVG(svgXML []byte, bounds geometry.Rect, color widge return } + // Honor the clip, as every other draw method on this canvas does. Without + // it an icon with no pixels on screen still emits its whole document into + // the scene. + if !c.isVisible(bounds) { + return + } + // Parse SVG (Level 1 document cache). doc := globalIconCache.getDoc(svgXML) if doc == nil { return } + // Keep the document inside the box it was given. The emitted geometry is + // positioned by a transform built from those bounds but is not otherwise + // constrained by them, so a document whose contents reach past its own + // viewBox — or a stroke that widens past it — paints over whatever sits + // around the icon. Every other primitive here is bounded by construction; + // a whole SVG is not. + clip := scene.NewRectShape(bounds.Min.X, bounds.Min.Y, bounds.Width(), bounds.Height()) + c.sc.PushClip(clip) + defer c.sc.PopClip() + // Vector path: emit SVG as scene geometry (paths + fills/strokes). // Resolution-independent — rendered at actual display resolution by the // GPU or CPU scene renderer. No bitmap pre-rasterization, no Level 2 diff --git a/internal/render/scene_canvas_svg_bounds_test.go b/internal/render/scene_canvas_svg_bounds_test.go new file mode 100644 index 0000000..ac48a45 --- /dev/null +++ b/internal/render/scene_canvas_svg_bounds_test.go @@ -0,0 +1,74 @@ +package render + +import ( + "testing" + + "github.com/gogpu/gg/scene" + "github.com/gogpu/ui/geometry" + "github.com/gogpu/ui/widget" +) + +// overflowingSVG draws a rectangle four times the size of its own viewBox. +// +// That is not a malformed document — an SVG may draw anywhere, and content +// outside the viewBox is clipped by the viewport in every renderer that follows +// the spec. Real icon sets depend on it: a stroke centered on the viewBox edge +// puts half its width outside, so "bleeds by a pixel" is the common case and a +// deliberate overflow is only the visible version of it. +const overflowingSVG = `` + + `` + +// An SVG must paint only inside the bounds it is handed. +// +// The vector path positions the document with a transform built from those +// bounds but does not otherwise constrain it, so anything the document draws +// past its viewBox lands on whatever surrounds the icon. It goes unseen +// wherever another widget paints afterwards and covers the spill — which is why +// it surfaces on the last icon in a row, or one against the edge of a window. +func TestSceneCanvasRenderSVGStaysInsideItsBounds(t *testing.T) { + globalIconCache.invalidateAll() + defer globalIconCache.invalidateAll() + + sc := scene.NewScene() + c := NewSceneCanvas(sc, 400, 400) + defer c.Close() + + box := geometry.NewRect(40, 40, 16, 16) // a 16x16 icon, as an app draws one + c.RenderSVG([]byte(overflowingSVG), box, widget.ColorBlack) + + // Scene.Bounds() is the union of the shapes and does not narrow for a clip, + // so the emitted commands are what has to be checked: the document must be + // wrapped in one. + var clips int + for _, tag := range sc.Flatten().Tags() { + if tag == scene.TagBeginClip { + clips++ + } + } + if clips == 0 { + t.Errorf("the SVG was emitted unbounded — %v of scene geometry with no clip around it, "+ + "so a document drawing past its viewBox paints over whatever surrounds the icon "+ + "(bounds given: %v)", sc.Bounds(), box) + } +} + +// Every other draw method on this canvas culls against the current clip. An +// icon with no pixels on screen should cost nothing, not emit its whole +// document into the scene. +func TestSceneCanvasRenderSVGHonorsTheClip(t *testing.T) { + globalIconCache.invalidateAll() + defer globalIconCache.invalidateAll() + + sc := scene.NewScene() + c := NewSceneCanvas(sc, 400, 400) + defer c.Close() + + c.PushClip(geometry.NewRect(0, 0, 20, 20)) + defer c.PopClip() + + before := sc.Version() + c.RenderSVG([]byte(overflowingSVG), geometry.NewRect(200, 200, 16, 16), widget.ColorBlack) + if sc.Version() != before { + t.Error("an icon entirely outside the clip still emitted scene commands") + } +}