fix: correct hatch draw order and COLORTHEME-aware fill colors#228
Merged
fix: correct hatch draw order and COLORTHEME-aware fill colors#228
Conversation
Fixes a family of rendering bugs around solid hatches resolved to the foreground pseudo-colour (ACI 7). Before: such hatches covered BYLAYER outlines in light mode (interior subdivisions of a tower DWG vanished behind white rectangles) and did not react to theme flips at all. - AcTrBatchedGroup: tag hatch / solid area fill meshes with `renderOrder = -1` so they draw below linework and text (matches AutoCAD's default SortentsTable ordering). Text glyph meshes go through the same batch path but are detected via `material.userData.isTextFill` and keep the default tier so they stay above lines. - AcTrFillMaterialManager: partition text glyph fills from hatch / solid area fills via a new `isTextFill` option. Text glyphs follow COLORTHEME inversion (`shouldTrackForeground`) so ACI 7 text stays legible on both themes; hatches follow the canvas background (`shouldTrackBackground`) so ACI 7 fills fuse with the paper in both themes. The two flags partition the material cache via `_text` and `_bgfill` key suffixes — appended only when applicable, so the common non-ACI-7 path keeps its keys bit-identical and its batches unchanged. - AcTrMaterialManager: add the `shouldTrackForeground` / `shouldTrackBackground` hooks so each primitive type can opt materials into either lifecycle independently. Introduce `changeBackground(color)` mirroring the existing `changeForeground` API. Defaults preserve current behaviour (foreground-aware, no background tracking). - AcTrStyleManager / AcTrStyleManagerOptions / AcTrRenderer: promote the canvas background colour to a tracked field (`currentBackgroundColor`). Writing it stores the value on the style manager options AND fires `changeBackground`, so materials created AFTER the flip are born with the correct colour while materials already in the cache are repainted. Closes the boot-in-dark edge case where ACI 7 hatches rendered solid white on the first frame until the user manually toggled the theme. - AcTrView2d: react to both WHITEBKCOLOR and COLORTHEME sysvars (previously only the former was bridged to the canvas). The Vue-side theme toggle (`useDark`) only flips COLORTHEME, so without this listener the canvas background stayed stale even as `changeForeground` fired for MText/lines. Both sysvars remain independently settable. - AcTrMTextRenderer: route MText glyph fills through the new `AcTrStyleManager.getMTextFillMaterial` so they are tagged as `isTextFill` and land in a separate cache slot from any hatch that happens to share colour + layer.
Owner
|
@pauloricardoma The correct way to control draw order is to use SORTENTSTABLE. For now, realdwg-web doesn't parse this object yet. So it is ok to handel it using your changes. Later, we will use SORTENTSTABLE to handle draw order after support parsing SORTENTSTABLE. https://help.autodesk.com/view/OARX/2024/ENU/?guid=GUID-462F4378-F850-4E89-90F2-3C1880F55779 |
Owner
|
@pauloricardoma There is one small issue on this change and you can fix it in next PR. Steps to reproduce it are as follows.
Result:
Expected Resut:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes a family of rendering bugs around solid hatches resolved to the foreground pseudo-colour (ACI 7):
batchConvertrendered with the default trait RGB on the first frame even when the theme was already flipped, only correcting themselves after a manual re-toggle.What changes
AcTrBatchedGroup— tag hatch / solid-fill meshes withrenderOrder = -1so they draw below linework and text (matches AutoCAD's defaultSortentsTableordering). Text glyph meshes go through the same batch path but keep the default tier viamaterial.userData.isTextFill.AcTrFillMaterialManager— newisTextFilloption partitions text glyph fills from hatch fills. Text glyphs follow COLORTHEME inversion (stay legible); hatches follow the canvas background (fuse with the paper). Cache keys gain_text/_bgfillsuffixes only when applicable, so the common non-ACI-7 path stays bit-identical.AcTrMaterialManager— newshouldTrackForeground/shouldTrackBackgroundhooks plus achangeBackground(color)method symmetric to the existingchangeForeground. Defaults preserve current behaviour; only the fill manager opts materials in.AcTrStyleManager/AcTrStyleManagerOptions/AcTrRenderer— newcurrentBackgroundColortracked field. Setter both stores the bg on the style manager options (so materials created later are born with it) and fireschangeBackground(so cached materials are repainted). Closes the boot-in-dark edge case.AcTrView2d— react to theCOLORTHEMEsysvar as well. Previously onlyWHITEBKCOLORwas bridged, but the Vue-side theme toggle (useDark) only flipsCOLORTHEME, so flipping the UI theme left the canvas background stale. Both sysvars remain independently settable.AcTrMTextRenderer— route MText glyph fills through the newAcTrStyleManager.getMTextFillMaterialso they land in theisTextFillpartition and never share a material instance with a hatch of the same colour + layer.Performance
Cache fragmentation is opt-in. The
_textand_bgfillsuffixes only attach when the relevant conditions match, so drawings without ACI 7 hatches or MText produce the exact same cache keys (and therefore the same batch count and draw calls) as before this PR.Depends on
mlightcad/realdwg-web#59 — preserves the DWG's truecolor when the entity carries both RGB and ACI index, so ACI 254 with a custom palette doesn't fall through to the ACI 7 path. The fix in this PR is correct on its own but only takes full effect once that upstream PR is released and bumped here.
Test plan
useDark) and open a DWG with ACI 7 hatches. First frame renders correctly — no manual re-toggle required.