Skip to content

Fix crashes when compositing masked layers inside a 3D context on Windows. - #1605

Merged
Hparty merged 3 commits into
mainfrom
bugfix/heefuture_layer_3d
Oct 29, 2026
Merged

Hparty merged 3 commits into
mainfrom
bugfix/heefuture_layer_3d

Conversation

@heefuture

@heefuture heefuture commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Two crashes hit Windows builds while rasterizing layers inside a 3D rendering context.

1. Member function pointer layout mismatch (ODR violation)

Layer3DContext.h declared LayerDrawFunc as a pointer to a Layer member function while Layer was only forward declared in that header. MSVC lays such a pointer out differently when the class is incomplete, so Layer3DContext.h and Layer.cpp (which includes Layer.h first) compiled addLayer() with different argument layouts. The context then invoked _drawFunc through a representation it was never given, crashing during 3D rasterization.

Fix: include tgfx/layers/Layer.h from Layer3DContext.h and drop the forward declaration, so LayerDrawFunc has one definition in every translation unit. No include cycle is introduced: Layer.h does not include Layer3DContext.h, and DrawArgs.h (the only other consumer) already sits below both.

2. Mask recorded while a 3D context is active

canPreserve3D() requires !hasValidMask(), so a masked layer inside a 3D subtree can only be a leaf. Layer3DContext::collectNodes emits it as a node, and Render3DContext::rasterLayer / Opaque3DContext::finishAndDrawTo set args.render3DContext before invoking the draw entry point. Every masked layer reached getMaskData with a non-null render3DContext, which contradicted the DEBUG_ASSERT and aborted debug builds.

Fix: keep the assertion and state the contract explicitly — a mask is always recorded in 2D on the current surface — then detach the mask draw from the 3D context at both getMaskData call sites (prepareMask and drawContourInternal). This matches the other intermediate artifacts (getContentContourImage, getLayerStyleSource, subtree cache), and any future change that feeds a mask back into the 3D path is caught by the assertion instead of silently slipping through.

Note: in release builds the leaked flag had no observable effect — render3DContext is only read by the two canPreserve3D() gates on the layer being traversed, and a mask subtree's own preserve3D nodes start their own context through drawChild. The only observable symptom was the debug abort.

Testing

  • MSVC Debug build of tgfx and TGFXFullTest_OpenGL
  • Full suite: 557 tests pass
  • New case LayerMaskTest.MaskUnderPreserve3DParent renders a preserve3D parent with a masked child and asserts the pixels inside and outside the mask, no screenshot baseline required. Verified it fails: removing the render3DContext detach at the call site makes it abort on the restored assertion.

Comment thread src/layers/Layer.cpp Outdated
DrawArgs maskArgs = args;
maskArgs.excludeEffects |= isContourMode;
maskArgs.backgroundHandler = BackgroundHandler::NoOp();
// The mask is always composited in 2D on the current surface. A non-null render3DContext would

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

断言的内容是对的,被违反的是契约而不是断言:mask 一律 2D 录制。

这条路径真实存在,而且不是误用:canPreserve3D() 要求 !hasValidMask(),所以带 mask 的层在 3D 子树里只能是叶子 → Layer3DContext::collectNodes 把叶子 emitNode → Render3DContext::rasterLayer(:246)设上 leafArgs.render3DContext 后调 drawWithFunc → Layer::drawLayer → prepareMask → getMaskData。

问题出在清标志的位置:maskArgs.render3DContext = nullptr 写在 getMaskPicture 里,在断言之后,所以为了让 Debug 不再 abort 只能连带把断言删掉。建议反过来:

  • 在 getMaskData 的两个调用点各清一次(prepareMask 的 :1192、drawContourInternal 的 :1625),DEBUG_ASSERT 保留;
  • 这样"mask 必须 2D 录制"成为显式契约,将来谁再把 mask 接进 3D 路径会被立刻抓到;getMaskPicture 里那行可以作为冗余保险保留或去掉。

顺带一点:描述里"release 下标志传进 mask 绘制导致 mask 不完整"我追不出来。render3DContext 只有两个读者(Layer.cpp:1672、:1692),判据都是"被遍历层自己的 canPreserve3D()";mask 本身是 3D 中间节点时走的是另起 context 的分支,mask 子树里的 preserve3D 节点由 drawChild 转去自己的 context。也就是说这次漏清在实际产物上无害,能观测到的只有 Debug abort。如果那条是从推断来的,建议把理由改成"和 1319/1414/1788 一样,中间产物一律 2D"。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

改为2D调用前把render3DContext清空

Comment thread src/layers/Layer.cpp

MaskData Layer::getMaskData(const DrawArgs& args, float scale,
const std::optional<Rect>& layerClipBounds) {
DEBUG_ASSERT(_mask != nullptr);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

这两个崩溃修复目前都没有用例覆盖(描述里也写了:恢复断言后 556 个用例全过)。建议在本 PR 补一个不需要截图基准的用例,而不是留到 follow-up:

  • preserve3D 父 + 带 mask 的子层,渲染后断言 mask 内外的像素;
  • Debug 下旧断言会直接打挂,release 下覆盖 mask 完整性;
  • 顺带保证 Layer3DContext.h 的 include 改动在 clang 侧也能编过(CI 覆盖不到 MSVC 那条 ODR 路径,但至少编译与全量用例要过)。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

保留断言,并增加了单测用例

@Hparty
Hparty merged commit 17ad188 into main Oct 29, 2026
15 checks passed
@Hparty
Hparty deleted the bugfix/heefuture_layer_3d branch October 29, 2026 04:21
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