scribble icon indicating copy to clipboard operation
scribble copied to clipboard

fix: guard repaint boundary keys via per-instance attach/detach

Open ehdnd opened this issue 4 months ago • 0 comments

Description

problem

  • Share of a single repaintBoundaryKey across multiple Scribble widgets caused Multiple widgets used the same GlobalKey when go_router kept two /notes/:noteId/edit pages alive via MaterialPage(maintainState: true, key: state.pageKey) (see it-contest repo, branch design/clean-dev-xodnd @f13a67f / dev @38f056b).
══╡ EXCEPTION CAUGHT BY WIDGETS LIBRARY
╞═══════════════════════════════════════════════════════════
The following assertion was thrown while finalizing the widget tree:
Multiple widgets used the same GlobalKey.
The key [GlobalKey#d6fd3] was used by multiple widgets. The parents of those widgets
were:
- CustomPaint(renderObject: RenderCustomPaint#f1798 NEEDS-PAINT)
- CustomPaint(renderObject: RenderCustomPaint#b3622)
A GlobalKey can only be specified on one widget at a time in the widget tree.

When the exception was thrown, this was the stack:
#0      BuildOwner._debugVerifyGlobalKeyReservation.<anonymous closure>.<anonymous
closure>.<anonymous closure> (package:flutter/src/widgets/framework.dart:3220:13)
#1      _LinkedHashMapMixin.forEach (dart:_compact_hash:764:13)
#2      BuildOwner._debugVerifyGlobalKeyReservation.<anonymous closure>.<anonymous
closure> (package:flutter/src/widgets/framework.dart:3164:20)
#3      _LinkedHashMapMixin.forEach (dart:_compact_hash:764:13)
#4      BuildOwner._debugVerifyGlobalKeyReservation.<anonymous closure>
(package:flutter/src/widgets/framework.dart:3158:36)
#5      BuildOwner._debugVerifyGlobalKeyReservation
(package:flutter/src/widgets/framework.dart:3228:6)
#6      BuildOwner.finalizeTree.<anonymous closure>
(package:flutter/src/widgets/framework.dart:3291:11)
#7      BuildOwner.finalizeTree (package:flutter/src/widgets/framework.dart:3378:8)
#8      WidgetsBinding.drawFrame (package:flutter/src/widgets/binding.dart:1247:19)
#9      RendererBinding._handlePersistentFrameCallback
(package:flutter/src/rendering/binding.dart:495:5)
#10     SchedulerBinding._invokeFrameCallback
(package:flutter/src/scheduler/binding.dart:1438:15)
#11     SchedulerBinding.handleDrawFrame
(package:flutter/src/scheduler/binding.dart:1351:9)
#12     SchedulerBinding._handleDrawFrame
(package:flutter/src/scheduler/binding.dart:1204:5)
#13     _invoke (dart:ui/hooks.dart:331:13)
#14     PlatformDispatcher._drawFrame (dart:ui/platform_dispatcher.dart:444:5)
#15     _drawFrame (dart:ui/hooks.dart:303:31)
═══════════════════════════════════════════════════════════════════════════════════════
═════════════

Another exception was thrown: Multiple widgets used the same GlobalKey.
Another exception was thrown: Multiple widgets used the same GlobalKey.
Another exception was thrown: Multiple widgets used the same GlobalKey.

how i solved

  • Converted Scribble to a StatefulWidget, giving each instance its own _localRepaintBoundaryKey and hooking the notifier through new attachRepaintBoundaryKey / detachRepaintBoundaryKey lifecycle methods.
  • Added a debug-only warning in ScribbleNotifier.renderImage whenever the legacy fallback key is used so integrators are nudged to adopt the new lifecycle before the fallback disappears.

how it works

  • ScribbleNotifier tracks the attached keys and falls back to the legacy key so existing code keeps working; renderImage now resolves to the most recently attached surface, which matches the single-canvas-per-notifier contract.

verification

  • Manual verification: dependency override from https://github.com/timcreatedit/scribble ref: main to https://github.com/ehdnd/scribble ref: fix/globalkey-collision inside https://github.com/tryCatchPing/it-contest (branches design/clean-dev-xodnd @f13a67f / dev @38f056b). Restored MaterialPage(maintainState: true)—which previously forced us to fallback to false—and confirmed linked-note navigation (link tap, backlinks panel, note list) no longer emits the GlobalKey assertion.
  • Automated verification: flutter test passes on Flutter 3.32.5 (FVM). The new widget test (Scribble does not throw when sharing a notifier across multiple instances) fails against pre-fix sources, confirming regression coverage.
  • ,, Attempts to build a minimal reproduction outside of the app were unsuccessful; the failure only appeared once the go_router + Riverpod keepAlive + maintainState stack was in place. If anyone manages to isolate a smaller example, happy to add it.

note for integrators

  • Custom ScribbleNotifierBase implementations must override attachRepaintBoundaryKey / detachRepaintBoundaryKey to keep renderImage working; consider this a behavior change for external notifiers.
    • In debug builds, continuing to rely on the legacy fallback GlobalKey now prints a warning so integrators notice the upcoming change.

Checklist

  • [x] My PR title is in the style of conventional commits
  • [x] All public facing APIs are documented with dartdoc
  • [x] I have added tests to cover my changes

ehdnd avatar Oct 16 '25 02:10 ehdnd