Skip to content

Fix DrawOnlyWidget rendering problems - #48

Merged
hajimehoshi merged 1 commit into
ebitengine:mainfrom
Vmarcelo49:main
Sep 26, 2026
Merged

hajimehoshi merged 1 commit into
ebitengine:mainfrom
Vmarcelo49:main

Conversation

@Vmarcelo49

@Vmarcelo49 Vmarcelo49 commented Dec 29, 2025 •

Copy link
Copy Markdown
Contributor

Fixes #47

The bug

DrawOnlyWidget created a nested widget, and widget skipped the drawing when the bounds of that
widget's layout item did not overlap the container body. Those bounds cannot tell whether the
output is visible, because the callback can draw outside them: in a grid cell the item is 18px
tall at the top of the cell, so the drawing disappeared as soon as that item scrolled out of the
window body, even though the cell was still visible.

The fix

Keep allocating the layout item, so that the layout does not depend on whether the drawing is
visible, and add the draw command without the bounds check. The clip rect already limits the output
to the visible area, and offscreen callbacks running is the accepted tradeoff for not having
explicit bounds. layout.go e widget.go ficam intocados.

Also in this PR: vector.DrawFilledRect is replaced with vector.FillRect (deprecated since
Ebitengine v2.9).

@hajimehoshi

Copy link
Copy Markdown
Member

This causes conflicts for now...

@Vmarcelo49

Copy link
Copy Markdown
Contributor Author

Solved this with an agent so i will paste its report:

Rebased onto main and squashed into a single commit. The conflict was only in the Context
struct: segStack/segStackIdx and the new inLayoutCallback field landed in the same spot.

Two things changed since the original patch:

  • the layout callback state is now saved and restored, so a nested GridCell does not clear it
    for the enclosing callback. Without that, a DrawOnlyWidget placed after an inner GridCell
    falls back to the old path and the widget disappears again.
  • added TestDrawOnlyWidgetInGridCellDuringScroll, which fails on the current main (the draw
    command for a still visible cell is culled once the cell's first layout item scrolls out) and
    passes with the fix.

The vector.DrawFilledRect -> vector.FillRect rename is unrelated cleanup (deprecated since
v2.9), and the visibility check now uses the clip rect instead of the container body bounds.
I can drop either one if you'd rather keep this on the fix only.

CI is green on Go 1.25/1.26/1.27 for Linux, macOS and Windows.

@hajimehoshi

Copy link
Copy Markdown
Member

I found a layout regression in the new inLayoutCallback handling (draw.go:211).

The flag remains true inside a Panel nested in GridCell, even though the panel has its own layout. As a result, a DrawOnlyWidget directly inside that panel skips allocating its row. Subsequent widgets move into the custom drawing's space, and the panel's scrollable content size is undercounted.

A minimal reproduction is this structure:

ctx.SetGridLayout(nil, []int{300})
ctx.GridCell(func(image.Rectangle) {
	ctx.SetGridLayout(nil, []int{250})
	ctx.Panel(func(layout debugui.ContainerLayout) {
		ctx.SetGridLayout(nil, []int{150})
		ctx.DrawOnlyWidget(func(*ebiten.Image) {})
		ctx.GridCell(func(bounds image.Rectangle) {
			// Inspect bounds.Min.Y - layout.BodyBounds.Min.Y.
		})
	})
})

I verified that the following cell's vertical offset is 159 pixels on the base and only 5 pixels with this PR. The same panel outside the enclosing GridCell still produces 159 pixels. A focused regression test passes on the base and fails with the PR; the existing test suite passes with the PR.

Could the flag be scoped to the layout that owns the callback, or saved/reset/restored when entering an independent container layout? That should preserve the scrolling fix without changing layout allocation inside nested panels.


Review comment authored by Codex (OpenAI), on behalf of @hajimehoshi.

@hajimehoshi

Copy link
Copy Markdown
Member

Following up on the nested-panel regression: I would favor preserving DrawOnlyWidget's existing layout allocation and removing only its bounds-based visibility check, without introducing inLayoutCallback.

The callback can draw beyond the row allocated by layoutNext(), so that row's bounds cannot reliably determine whether its output is visible. The current clip rectangle already limits the drawing. For example:

func (c *Context) DrawOnlyWidget(f func(screen *ebiten.Image)) {
	_ = c.wrapEventHandlerAndError(func() (EventHandler, error) {
		if _, err := c.layoutNext(); err != nil {
			return nil, err
		}

		c.setClip(c.clipRect())
		defer c.setClip(unclippedRect)

		cmd := c.appendCommand(commandDraw)
		cmd.draw.f = f
		return nil, nil
	})
}

This would keep row allocation consistent for standalone calls and calls inside nested panels, while allowing drawing in a partially visible grid cell to continue. It also avoids making layout allocation depend on whether an ancestor is currently running a layout callback. No change to other widgets' visibility checks is needed for this approach.

The tradeoff is that offscreen callbacks would still execute. The proposed inLayoutCallback path already does that for the reported grid-cell case; reliable culling would require explicit bounds for the callback's drawing.

This is a suggested alternative, not a tested patch. I would validate it against both the scrolling regression and the nested-panel layout regression described above.


Comment authored by Codex (OpenAI), on behalf of @hajimehoshi.

@Vmarcelo49
Vmarcelo49 force-pushed the main branch 2 times, most recently from 9b81555 to 7059efa Compare September 26, 2026 18:39
@hajimehoshi

Copy link
Copy Markdown
Member

The simplified implementation looks good. Could you remove the two added tests and the DrawCommandCount helper from this PR?

The command-count test exposes internal drawing bookkeeping and requires exactly five queued commands, without verifying the visible rendering. The nested-panel test uses public behavior, but it primarily guards against the now-removed inLayoutCallback approach. For this small fix, manually checking the original scrolling reproduction is sufficient; these additional tests are not necessary.

Please also fix the import grouping in debugui_test.go: standard library first, external dependencies second, and the current module (github.com/ebitengine/debugui) in a separate final group. Removing these tests should make the newly added Ebitengine import unused, in which case it can simply be removed.


Comment authored by Codex (OpenAI), on behalf of @hajimehoshi.

DrawOnlyWidget created a nested widget, and that widget was culled when the
bounds of its layout item did not overlap the container body. The callback
can draw outside those bounds, so they cannot tell whether the output is
visible: in a grid cell the item has the default height at the top of the
cell, and the drawing disappeared once that item scrolled out while the
cell was still visible.

Allocate the layout item as before, so that the layout does not depend on
whether the drawing is visible, and add the draw command without the
visibility check. The clip rect already limits the output to the visible
area.

Also replace the deprecated vector.DrawFilledRect with vector.FillRect.

Fixes ebitengine#47

@hajimehoshi hajimehoshi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks!

@hajimehoshi
hajimehoshi merged commit c97281f into ebitengine:main Sep 26, 2026
9 checks passed
hajimehoshi pushed a commit that referenced this pull request Sep 26, 2026
DrawOnlyWidget created a nested widget, and that widget was culled when the
bounds of its layout item did not overlap the container body. The callback
can draw outside those bounds, so they cannot tell whether the output is
visible: in a grid cell the item has the default height at the top of the
cell, and the drawing disappeared once that item scrolled out while the
cell was still visible.

Allocate the layout item as before, so that the layout does not depend on
whether the drawing is visible, and add the draw command without the
visibility check. The clip rect already limits the output to the visible
area.

Also replace the deprecated vector.DrawFilledRect with vector.FillRect.

Closes #47
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.

Widget not rendered on window scoll

2 participants