feat(2d): add FilledSpriteAssembler and sprite filled mode support#2989
feat(2d): add FilledSpriteAssembler and sprite filled mode support#2989cptbtptpbcptdtptp wants to merge 2 commits into
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
WalkthroughThis PR adds filled-mode sprite rendering to the Galacean 2D engine. It introduces new enums ( ChangesFilled Sprite Rendering
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev/2.0 #2989 +/- ##
===========================================
- Coverage 78.25% 77.62% -0.64%
===========================================
Files 900 903 +3
Lines 99234 99970 +736
Branches 10172 10178 +6
===========================================
- Hits 77657 77599 -58
- Misses 21406 22200 +794
Partials 171 171
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
packages/core/src/2d/assembler/FilledSpriteAssembler.ts (3)
113-115: 💤 Low value
@ts-ignorehides an interface gap onrenderer._bounds.The
_boundsfield isn't declared onISpriteRenderer, so this access requires@ts-ignore. Other assemblers presumably need the same hatch. Consider wideningISpriteRendererto include_bounds: BoundingBox(or expose a small_setBounds(box)method) so the suppression can be removed and the interface accurately reflects what the assemblers depend on.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/2d/assembler/FilledSpriteAssembler.ts` around lines 113 - 115, The `@ts-ignore` exists because assemblers access renderer._bounds which isn't declared on ISpriteRenderer; update the renderer interface (ISpriteRenderer) to include a private/internal _bounds: BoundingBox (or add a small method like _setBounds(box: BoundingBox) and use that) so FilledSpriteAssembler (where BoundingBox.transform(renderer._getBounds(), modelMatrix, renderer._bounds) is called) no longer needs `@ts-ignore`; change the interface accordingly and remove the suppression from FilledSpriteAssembler (and any other assemblers using _bounds).
137-140: 💤 Low valueExtract repeated
amount <= 0.001short-circuit and the magic threshold.The same
if (amount <= 0.001) { renderer._subChunk.indices.length = 0; return; }guard is repeated in 4_filled*methods, and the threshold itself is a magic number. Consider hoisting both into a single named constant + helper (or moving the early-out up intoupdatePositionsso the per-mode functions don't have to duplicate it).Also applies to: 222-225, 288-291, 383-386
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/2d/assembler/FilledSpriteAssembler.ts` around lines 137 - 140, Extract the repeated small-amount guard into a named constant and single helper (or move the early-out into updatePositions) to remove duplication: define a descriptive constant (e.g. MIN_FILL_AMOUNT = 0.001) in FilledSpriteAssembler and replace the four occurrences of the literal check (the lines in the _filled* methods that do `if (amount <= 0.001) { renderer._subChunk.indices.length = 0; return; }`) with a call to a new helper like shouldSkipFill(amount, renderer) or performSkipFillIfNeeded(amount, renderer) so the threshold is centralized and the index-clear/return logic is not duplicated across _filled* methods. Ensure you update all four locations mentioned (the guards at the shown snippet and the ones around lines 222-225, 288-291, 383-386) and keep behavior identical.
244-271: ⚡ Quick winRadial handlers silently no-op on unexpected origins.
_filledRadial90only handles the 4 corner origins,_filledRadial180and_filledRadial360only handle the 4 edge origins. Thedefault: break;branches leave_inPositions/_inUVspopulated from a previous invocation (radial 90/180) orstartAngle = 0(radial 360), producing visually wrong geometry without any indication of misuse.Since
SpriteRenderer.filledOriginsetter does not validate againstfilledMode, a user can put the renderer in a bad state. Consider either:
- Validating in
SpriteRenderer.filledOriginsetter (reject/coerce origins that don't match the current mode), or- Adding an explicit warning/
Logger.warnin the default branch here.Also applies to: 320-371, 389-404
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/2d/assembler/FilledSpriteAssembler.ts` around lines 244 - 271, The switch default branches in _filledRadial90, _filledRadial180 and _filledRadial360 silently leave inPositions/inUVs from prior calls causing wrong geometry; update each default case to log a warning (use Logger.warn or the project logger) mentioning the function name and the invalid origin, and then clear/reset the arrays (inPositions and inUVs) or set them to safe defaults so stale data isn't reused; alternatively, add validation/coercion in the SpriteRenderer.filledOrigin setter to reject or coerce origins incompatible with the current filledMode, but if you choose the logger approach make sure to reference the origin value and the method name in the message.packages/core/src/2d/sprite/SpriteRenderer.ts (1)
179-186: ⚡ Quick winConsider validating
filledOriginagainst the activefilledMode.The setter accepts any
SpriteFilledOrigin, but eachfilledModeonly renders correctly for a subset of origins (Horizontal: Left/Right; Vertical: Top/Bottom; Radial90: corners; Radial180/360: edge midpoints). Combined with the silentdefault: break;branches inFilledSpriteAssembler, an invalid combination produces broken geometry with no diagnostic.Validating here (e.g.,
Logger.warnand coercing to a valid origin, or clamping based on_filledMode) would help users catch misuse early. Same applies to thefilledModesetter when the previously-set origin is no longer valid for the new mode.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/2d/sprite/SpriteRenderer.ts` around lines 179 - 186, The filledOrigin setter currently accepts any SpriteFilledOrigin even when incompatible with the active _filledMode, which can produce broken geometry; update the set filledOrigin(value: SpriteFilledOrigin) to validate value against the current _filledMode (e.g., allowed origins for Horizontal = Left/Right, Vertical = Top/Bottom, Radial90 = corners, Radial180/360 = edge midpoints), and if invalid call Logger.warn with a clear message and coerce/clamp to a valid origin before assigning _filledOrigin and setting the dirty flag (_dirtyUpdateFlag |= SpriteRendererUpdateFlags.WorldVolumeAndUV) for SpriteDrawMode.Filled; also mirror this validation in the filledMode setter so when changing _filledMode you adjust or coerce the existing _filledOrigin (and warn) to a compatible value to avoid relying on the silent default branches in FilledSpriteAssembler.packages/core/src/2d/assembler/ISpriteRenderer.ts (1)
20-20: ⚡ Quick winConsider renaming
filledClockWise→filledClockwise."Clockwise" is a single English word; splitting the W creates an unusual public API name. Since this property is also exposed via
SpriteRendererand is part of a 2.0 release, normalizing it now avoids a breaking rename later. Same rename should be applied inSpriteRenderer.ts(private field, getter/setter) andFilledSpriteAssembler.ts(read site).♻️ Suggested rename
- filledClockWise?: boolean; + filledClockwise?: boolean;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/2d/assembler/ISpriteRenderer.ts` at line 20, Rename the public property and all usages from filledClockWise to filledClockwise: update the ISpriteRenderer interface property (filledClockWise → filledClockwise), rename the private field and its getter/setter in SpriteRenderer (ensure method names and internal references use filledClockwise), and update the read site in FilledSpriteAssembler to read the new property name; keep behavior unchanged and run tests to catch any missed references.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/2d/assembler/FilledSpriteAssembler.ts`:
- Around line 117-119: The FilledSpriteAssembler.updateUVs is a no-op which
leaves UVs stale when atlasRegion changes because SpriteRenderer._onSpriteChange
sets only SpriteRendererUpdateFlags.UV for SpriteModifyFlags.atlasRegion and
_render will call updateUVs (skipped updatePositions); fix this by changing
SpriteRenderer._onSpriteChange to treat atlasRegion the same as other
UV-affecting changes for filled mode: when atlasRegion is detected, set the
update flags to WorldVolumeAndUV (or include WorldVolume) instead of only UV so
FilledSpriteAssembler.updatePositions runs and recomputes baked UVs; update
references to SpriteModifyFlags.atlasRegion and SpriteRendererUpdateFlags.*
accordingly.
In `@packages/core/src/2d/sprite/SpriteRenderer.ts`:
- Around line 160-170: The filledMode setter currently sets _filledOrigin to
Bottom for all non-Radial90 modes which makes Horizontal default to an invalid
Bottom origin; update the logic in the set filledMode(value: SpriteFilledMode)
to choose a mode-appropriate default: when value === SpriteFilledMode.Horizontal
set _filledOrigin to SpriteFilledOrigin.Left (so
FilledSpriteAssembler._filledLinear sees originIsStart true), when value ===
SpriteFilledMode.Radial90 set it to SpriteFilledOrigin.BottomLeft, otherwise
keep SpriteFilledOrigin.Bottom; keep the existing branch that sets
_dirtyUpdateFlag |= SpriteRendererUpdateFlags.WorldVolumeAndUV when
this._drawMode === SpriteDrawMode.Filled.
---
Nitpick comments:
In `@packages/core/src/2d/assembler/FilledSpriteAssembler.ts`:
- Around line 113-115: The `@ts-ignore` exists because assemblers access
renderer._bounds which isn't declared on ISpriteRenderer; update the renderer
interface (ISpriteRenderer) to include a private/internal _bounds: BoundingBox
(or add a small method like _setBounds(box: BoundingBox) and use that) so
FilledSpriteAssembler (where BoundingBox.transform(renderer._getBounds(),
modelMatrix, renderer._bounds) is called) no longer needs `@ts-ignore`; change the
interface accordingly and remove the suppression from FilledSpriteAssembler (and
any other assemblers using _bounds).
- Around line 137-140: Extract the repeated small-amount guard into a named
constant and single helper (or move the early-out into updatePositions) to
remove duplication: define a descriptive constant (e.g. MIN_FILL_AMOUNT = 0.001)
in FilledSpriteAssembler and replace the four occurrences of the literal check
(the lines in the _filled* methods that do `if (amount <= 0.001) {
renderer._subChunk.indices.length = 0; return; }`) with a call to a new helper
like shouldSkipFill(amount, renderer) or performSkipFillIfNeeded(amount,
renderer) so the threshold is centralized and the index-clear/return logic is
not duplicated across _filled* methods. Ensure you update all four locations
mentioned (the guards at the shown snippet and the ones around lines 222-225,
288-291, 383-386) and keep behavior identical.
- Around line 244-271: The switch default branches in _filledRadial90,
_filledRadial180 and _filledRadial360 silently leave inPositions/inUVs from
prior calls causing wrong geometry; update each default case to log a warning
(use Logger.warn or the project logger) mentioning the function name and the
invalid origin, and then clear/reset the arrays (inPositions and inUVs) or set
them to safe defaults so stale data isn't reused; alternatively, add
validation/coercion in the SpriteRenderer.filledOrigin setter to reject or
coerce origins incompatible with the current filledMode, but if you choose the
logger approach make sure to reference the origin value and the method name in
the message.
In `@packages/core/src/2d/assembler/ISpriteRenderer.ts`:
- Line 20: Rename the public property and all usages from filledClockWise to
filledClockwise: update the ISpriteRenderer interface property (filledClockWise
→ filledClockwise), rename the private field and its getter/setter in
SpriteRenderer (ensure method names and internal references use
filledClockwise), and update the read site in FilledSpriteAssembler to read the
new property name; keep behavior unchanged and run tests to catch any missed
references.
In `@packages/core/src/2d/sprite/SpriteRenderer.ts`:
- Around line 179-186: The filledOrigin setter currently accepts any
SpriteFilledOrigin even when incompatible with the active _filledMode, which can
produce broken geometry; update the set filledOrigin(value: SpriteFilledOrigin)
to validate value against the current _filledMode (e.g., allowed origins for
Horizontal = Left/Right, Vertical = Top/Bottom, Radial90 = corners,
Radial180/360 = edge midpoints), and if invalid call Logger.warn with a clear
message and coerce/clamp to a valid origin before assigning _filledOrigin and
setting the dirty flag (_dirtyUpdateFlag |=
SpriteRendererUpdateFlags.WorldVolumeAndUV) for SpriteDrawMode.Filled; also
mirror this validation in the filledMode setter so when changing _filledMode you
adjust or coerce the existing _filledOrigin (and warn) to a compatible value to
avoid relying on the silent default branches in FilledSpriteAssembler.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 24f0fb05-7587-4d34-9703-6bb15d3ffab5
📒 Files selected for processing (7)
packages/core/src/2d/assembler/FilledSpriteAssembler.tspackages/core/src/2d/assembler/ISpriteRenderer.tspackages/core/src/2d/enums/SpriteDrawMode.tspackages/core/src/2d/enums/SpriteFilledMode.tspackages/core/src/2d/enums/SpriteFilledOrigin.tspackages/core/src/2d/index.tspackages/core/src/2d/sprite/SpriteRenderer.ts
| static updateUVs(renderer: ISpriteRenderer): void { | ||
| // UVs are computed in updatePositions. | ||
| } |
There was a problem hiding this comment.
Empty updateUVs leaves UVs stale on atlas-region changes.
updateUVs is a no-op because UVs are baked inside updatePositions. However, SpriteRenderer._onSpriteChange handles SpriteModifyFlags.atlasRegion by setting only SpriteRendererUpdateFlags.UV (no WorldVolume). In _render, this triggers updateUVs while updatePositions is skipped — so for filled mode the UVs are never refreshed when the sprite's atlas region changes (e.g., dynamic atlas repacking). Other assemblers don't have this regression because they have a real updateUVs.
Two reasonable fixes:
- In
SpriteRenderer._onSpriteChange, treatatlasRegionlike other UV-affecting events for filled mode (setWorldVolumeAndUV), or - Implement a real UV-only path here (extract the UV computation from each
_filled*function so it can be invoked independently).
Option 1 is the minimal fix.
🛠 Minimal fix in SpriteRenderer._onSpriteChange
case SpriteModifyFlags.atlasRegion:
- this._dirtyUpdateFlag |= SpriteRendererUpdateFlags.UV;
+ this._dirtyUpdateFlag |=
+ this._drawMode === SpriteDrawMode.Filled
+ ? SpriteRendererUpdateFlags.WorldVolumeAndUV
+ : SpriteRendererUpdateFlags.UV;
break;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/2d/assembler/FilledSpriteAssembler.ts` around lines 117 -
119, The FilledSpriteAssembler.updateUVs is a no-op which leaves UVs stale when
atlasRegion changes because SpriteRenderer._onSpriteChange sets only
SpriteRendererUpdateFlags.UV for SpriteModifyFlags.atlasRegion and _render will
call updateUVs (skipped updatePositions); fix this by changing
SpriteRenderer._onSpriteChange to treat atlasRegion the same as other
UV-affecting changes for filled mode: when atlasRegion is detected, set the
update flags to WorldVolumeAndUV (or include WorldVolume) instead of only UV so
FilledSpriteAssembler.updatePositions runs and recomputes baked UVs; update
references to SpriteModifyFlags.atlasRegion and SpriteRendererUpdateFlags.*
accordingly.
| if (isHorizontal) { | ||
| const originIsStart = renderer.filledOrigin === SpriteFilledOrigin.Left; | ||
| const startX = originIsStart ? lPosLB.x : lPosRB.x - (lPosRB.x - lPosLB.x) * amount; | ||
| const endX = originIsStart ? lPosLB.x + (lPosRB.x - lPosLB.x) * amount : lPosRB.x; | ||
| const startU = originIsStart ? left : right - (right - left) * amount; | ||
| const endU = originIsStart ? left + (right - left) * amount : right; | ||
| (x0 = startX), (y0 = lPosLB.y), (u0 = startU), (v0 = bottom); | ||
| (x1 = endX), (y1 = lPosRB.y), (u1 = endU), (v1 = bottom); | ||
| (x2 = startX), (y2 = lPosLT.y), (u2 = startU), (v2 = top); | ||
| (x3 = endX), (y3 = lPosRT.y), (u3 = endU), (v3 = top); | ||
| } else { | ||
| const originIsStart = renderer.filledOrigin === SpriteFilledOrigin.Bottom; | ||
| const startY = originIsStart ? lPosLB.y : lPosLT.y - (lPosLT.y - lPosLB.y) * amount; | ||
| const endY = originIsStart ? lPosLB.y + (lPosLT.y - lPosLB.y) * amount : lPosLT.y; | ||
| const startV = originIsStart ? bottom : top - (top - bottom) * amount; | ||
| const endV = originIsStart ? bottom + (top - bottom) * amount : top; | ||
| (x0 = lPosLB.x), (y0 = startY), (u0 = left), (v0 = startV); | ||
| (x1 = lPosRB.x), (y1 = startY), (u1 = right), (v1 = startV); | ||
| (x2 = lPosLT.x), (y2 = endY), (u2 = left), (v2 = endV); | ||
| (x3 = lPosRT.x), (y3 = endY), (u3 = right), (v3 = endV); | ||
| } |
There was a problem hiding this comment.
_filledLinear silently treats any non-Left/non-Bottom origin as the opposite end.
originIsStart = renderer.filledOrigin === SpriteFilledOrigin.Left (resp. Bottom) means any other origin value — including corner origins or an unrelated default — quietly behaves as Right (resp. Top). Combined with the default-origin choice in SpriteRenderer.filledMode setter (see comment there), filledMode = Horizontal currently fills from the right side by default, which is almost certainly not intended.
Once the default-origin issue in SpriteRenderer is fixed, consider tightening the check here to be explicit (e.g., assert/validate the origin matches the mode, or compare against both valid values and warn otherwise). At minimum, an explicit comparison documents intent better than a single-sided equality.
| set filledMode(value: SpriteFilledMode) { | ||
| if (this._filledMode !== value) { | ||
| this._filledMode = value; | ||
| // Reset origin to a valid default for the new mode | ||
| this._filledOrigin = | ||
| value === SpriteFilledMode.Radial90 ? SpriteFilledOrigin.BottomLeft : SpriteFilledOrigin.Bottom; | ||
| if (this._drawMode === SpriteDrawMode.Filled) { | ||
| this._dirtyUpdateFlag |= SpriteRendererUpdateFlags.WorldVolumeAndUV; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
filledMode setter picks an invalid default origin for Horizontal.
The expression value === SpriteFilledMode.Radial90 ? BottomLeft : Bottom assigns SpriteFilledOrigin.Bottom to every non-Radial90 mode, including Horizontal. For horizontal fills, only Left/Right are meaningful origins, and in FilledSpriteAssembler._filledLinear:
const originIsStart = renderer.filledOrigin === SpriteFilledOrigin.Left;So when a user writes renderer.filledMode = SpriteFilledMode.Horizontal, the renderer ends up with origin = Bottom → originIsStart = false → fills from the right side. That's almost certainly not the intended default.
🛠 Suggested fix
set filledMode(value: SpriteFilledMode) {
if (this._filledMode !== value) {
this._filledMode = value;
// Reset origin to a valid default for the new mode
- this._filledOrigin =
- value === SpriteFilledMode.Radial90 ? SpriteFilledOrigin.BottomLeft : SpriteFilledOrigin.Bottom;
+ switch (value) {
+ case SpriteFilledMode.Horizontal:
+ this._filledOrigin = SpriteFilledOrigin.Left;
+ break;
+ case SpriteFilledMode.Radial90:
+ this._filledOrigin = SpriteFilledOrigin.BottomLeft;
+ break;
+ default:
+ this._filledOrigin = SpriteFilledOrigin.Bottom;
+ break;
+ }
if (this._drawMode === SpriteDrawMode.Filled) {
this._dirtyUpdateFlag |= SpriteRendererUpdateFlags.WorldVolumeAndUV;
}
}
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/2d/sprite/SpriteRenderer.ts` around lines 160 - 170, The
filledMode setter currently sets _filledOrigin to Bottom for all non-Radial90
modes which makes Horizontal default to an invalid Bottom origin; update the
logic in the set filledMode(value: SpriteFilledMode) to choose a
mode-appropriate default: when value === SpriteFilledMode.Horizontal set
_filledOrigin to SpriteFilledOrigin.Left (so FilledSpriteAssembler._filledLinear
sees originIsStart true), when value === SpriteFilledMode.Radial90 set it to
SpriteFilledOrigin.BottomLeft, otherwise keep SpriteFilledOrigin.Bottom; keep
the existing branch that sets _dirtyUpdateFlag |=
SpriteRendererUpdateFlags.WorldVolumeAndUV when this._drawMode ===
SpriteDrawMode.Filled.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
GuoLei1990
left a comment
GuoLei1990
left a comment
There was a problem hiding this comment.
已关闭问题清单
| 问题 | 状态 |
|---|---|
| spriteUVs[0]/[15] 依赖 PR #2990 的 16-point UV | ✅ 已关闭（依赖 #2990，合并顺序保证） |
| updateColor 遍历全 16 slot 写颜色 | ✅ 已关闭（未使用顶点无 indices，不影响渲染） |
总结
新增 FilledSpriteAssembler，支持 Horizontal/Vertical/Radial90/Radial180/Radial360 五种填充模式，对应 Unity Image.fillMethod + Image.fillAmount 语义。Radial 模式通过参数化四边形原点旋转统一 4 种 corner origin，设计简洁。613 行实现量可观但逻辑清晰。
[P2] _filledLinear 和所有 Radial 方法在 atlasRotated=true 时 UV 命名语义颠倒
_filledLinear 中：
const { x: left, y: bottom } = spriteUVs[0];
const { x: right, y: top } = spriteUVs[15];根据 PR #2990 的 _updateUVs：
- non-rotated：
uvs[0] = (left, bottom)，uvs[15] = (right, top)— 变量命名正确 - atlasRotated：
uvs[0] = (left, top)，uvs[15] = (right, bottom)—bottom/top实际含义颠倒
当 atlas 旋转时，_filledLinear 中 startV/endV 插值方向会反转（bottom 和 top 互换），导致 vertical fill 显示倒序。Radial 系列同理（uvLB/uvLT 命名在 rotated 时错误）。
修复方向：在 _filledLinear/_filledRadial* 的 UV 取点处，不假设 uvs[0].y = bottom，而是从 spriteUVs 读取具体的 x/y 值并命名为 uLeft/uRight/vBottom/vTop（不加 atlas 语义假设），或在每个方法开头区分 atlasRotated。
P2，不阻塞合并（当前代码在 non-rotated 场景正确），但建议在合入前修复以避免 atlasRotated + filledMode 组合场景出现视觉 bug。
GuoLei1990
left a comment
GuoLei1990
left a comment
There was a problem hiding this comment.
已关闭问题清单
| 问题 | 状态 |
|---|---|
| spriteUVs[0]/[15] 依赖 PR #2990 的 16-point UV | ✅ 已关闭（依赖 #2990，合并顺序保证） |
| updateColor 遍历全 16 slot 写颜色 | ✅ 已关闭（未使用顶点无 indices，不影响渲染） |
总结
新增 FilledSpriteAssembler，支持 Horizontal/Vertical/Radial90/Radial180/Radial360 五种填充模式，对应 Unity Image.fillMethod + Image.fillAmount 语义。Radial 模式通过参数化四边形原点旋转统一 4 种 corner origin，设计简洁。
问题
[P2] _filledLinear 和所有 Radial 方法在 atlasRotated=true 时 UV 命名语义颠倒
_filledLinear 中：
const { x: left, y: bottom } = spriteUVs[0];
const { x: right, y: top } = spriteUVs[15];根据 PR #2990 的 _updateUVs：
- non-rotated：
uvs[0] = (left, bottom)，uvs[15] = (right, top)— 变量命名正确 - atlasRotated：
uvs[0] = (left, top)，uvs[15] = (right, bottom)—bottom/top实际含义颠倒
当 atlas 旋转时，_filledLinear 中 startV/endV 插值方向会反转，导致 vertical fill 显示倒序。Radial 系列同理（uvLB/uvLT 命名在 rotated 时错误）。
修复方向：在 _filledLinear/_filledRadial* 的 UV 取点处，不假设 uvs[0].y = bottom，而是直接使用 uLeft/uRight/vBottom/vTop（从 non-rotated 语义基准取值），或在每个方法开头区分 atlasRotated。
P2，当前代码在 non-rotated 场景正确，atlasRotated + filledMode 组合场景会有视觉 bug。此问题上轮 review 已提出，尚未修复。
GuoLei1990
left a comment
GuoLei1990
left a comment
There was a problem hiding this comment.
增量审查（2026-05-15）
已关闭问题清单
| 问题 | 状态 |
|---|---|
spriteUVs[0]/[15] 依赖 PR #2990 的 16-point UV |
✅ 已关闭（依赖 #2990，合并顺序保证） |
updateColor 遍历全 16 slot 写颜色 |
✅ 已关闭（未使用顶点无 indices，不影响渲染） |
总结
新增 FilledSpriteAssembler，支持 Horizontal/Vertical/Radial90/Radial180/Radial360 五种填充模式，与 Unity Image.FillMethod 语义对齐。Radial 模式通过参数化四边形原点旋转统一四种 corner origin，设计简洁。功能完整，依赖 PR #2990 先合入。
问题
[P2] _filledLinear 和所有 Radial 方法在 atlasRotated=true 时 UV 命名语义颠倒
_filledLinear 中：
const { x: left, y: bottom } = spriteUVs[0];
const { x: right, y: top } = spriteUVs[15];根据 PR #2990 的 _updateUVs：
- non-rotated：
uvs[0] = (left, bottom)，uvs[15] = (right, top)— 变量命名正确 - atlasRotated：
uvs[0] = (left, top)，uvs[15] = (right, bottom)—bottom/top实际含义颠倒
当 atlas 旋转时，_filledLinear 的 startV/endV 插值方向会反转，导致 vertical fill 显示倒序。Radial 系列同理（uvLB/uvLT 命名在 rotated 时语义错误）。
此问题上轮 review 已提出，不阻塞当前 non-rotated 场景。atlasRotated + filledMode 组合场景会有视觉 bug，建议后续修复或在文档中注明限制。P2，不阻塞合入（需先合 #2990）。
GuoLei1990
left a comment
GuoLei1990
left a comment
There was a problem hiding this comment.
总结
新增 FilledSpriteAssembler，支持 Horizontal/Vertical/Radial90/Radial180/Radial360 五种填充模式，对应 Unity SpriteRenderer.drawMode = Filled 的语义。这是 2D 进度条/冷却圈等常见 UI 需求的标配功能。
架构设计合理：静态分配的 scratch buffer（_worldPositions/_uvs/_outPositions/_outUVs）避免 per-frame GC，_radialCut 作为核心几何裁切原语被 Radial90/180/360 复用。
问题
[P2] FilledSpriteAssembler.ts:56（resetData）— allocate 16 vertices，但 linear 模式只用 4 个
const subChunk = manager.allocateSubChunk(16);Linear（Horizontal/Vertical）填充只需 4 顶点（1 quad），但每次 resetData 都分配了 16 个顶点槽。对于频繁变更 filledAmount 的进度条场景，这意味着顶点 buffer 利用率只有 25%。Radial360 最多 4 个 quadrant × 4 顶点 = 16，但 Radial90 最多只需 4 顶点。
建议：如果顶点数可以在 resetData 时已知（filledMode 在组件上是可枚举的），按 mode 分配精确数量；如果 mode 动态可变，至少可以按 min(mode 最大顶点数, 16) 分配。不过这属于优化，不阻塞合入。
[P2] FilledSpriteAssembler.ts:142（_filledLinear）— 使用 spriteUVs[0] 和 spriteUVs[15] 的 x/y 作为 UV 的左右上下
const { x: left, y: bottom } = spriteUVs[0];
const { x: right, y: top } = spriteUVs[15];当 atlasRotated = true 时，_updateUVs 填充的是旋转后的 UV 坐标，此时 spriteUVs[0]、spriteUVs[15] 代表的是 atlas 中实际的 UV 角点，但这两个点的 x/y 分量在旋转 atlas 下不再分别对应"水平""垂直"方向。_filledLinear 用 left/right 驱动 U 插值、top/bottom 驱动 V 插值，在旋转 atlas 下可能产生错误的 UV 映射。
此问题属于"rotated atlas + filled mode"的组合场景，当前 atlas 支持尚在 PR #2990 中引入，两个 PR 之间存在潜在交叉问题。建议在文档或代码中明确此组合的支持状态。
[P3] FilledSpriteAssembler.ts — SpriteFilledOrigin.Bottom 在 Radial360 的 startAngle 为 270°
Radial360 以 Right = 0° 为基准，Top = 90°，Left = 180°，Bottom = 270°。这与 Unity 的 SpriteRenderer.fillOrigin 起始角度枚举一致，正确。
可合入
GuoLei1990
left a comment
GuoLei1990
left a comment
There was a problem hiding this comment.
增量审查（2026-05-15）
已关闭问题清单
| 问题 | 状态 |
|---|---|
spriteUVs[0]/[15] 依赖 PR #2990 的 16-point UV |
✅ 已关闭（依赖 #2990，合并顺序保证） |
updateColor 遍历全 16 slot 写颜色 |
✅ 已关闭（未使用顶点无 indices，不影响渲染） |
总结
新增 FilledSpriteAssembler，支持 Horizontal/Vertical/Radial90/Radial180/Radial360 五种填充模式，对标 Unity Image.fillMethod。613 行新代码覆盖了所有模式——radial 填充的扇形裁切几何本身就需要这个复杂度，没有冗余。_radialCut 作为核心几何裁切原语被 Radial90/180/360 复用，设计清晰。静态分配 scratch buffer（_worldPositions/_uvs/_outPositions/_outUVs）避免 per-frame GC，对热路径有利。
问题
[P2] FilledSpriteAssembler.ts:_filledLinear:130-168 — 本地坐标系下的 UV 与 Sprite._uvs[0]/[15] 的对应关系依赖 #2990 的坐标轴方向
_filledLinear 中取 spriteUVs[0].x 作 left、spriteUVs[15].y 作 top，这只在 #2990 的 non-rotated 分支中成立（uvs[0] = LB, uvs[15] = RT）。当 atlasRotated = true 时，uvs[0] 和 uvs[15] 的语义在 #2990 中仍是 LB/RT（因为 _updateUVs 已在 UV 层面处理了旋转映射，Assembler 无需感知），所以实际上是正确的。建议在代码中加一句注释说明："Assembler-visible UV grid is always in display-space (rotated atlas handled inside _updateUVs), so [0]=LB / [15]=RT regardless of atlasRotated."
[P2] updatePositions 中 BoundingBox.transform 的 @ts-ignore
renderer._bounds 存在于 SpriteRenderer 实现上但未在 ISpriteRenderer 接口中声明，导致需要 // @ts-ignore。这是接口不完整的信号——要么在 ISpriteRenderer 中声明 _bounds，要么让 Assembler 接受 SpriteRenderer 类型（只有 sprite 相关组件才用这个 assembler）。
无阻塞问题，此 PR 依赖 #2990 先合入，LGTM。
2 participants
Summary
Adds support for Sprite Filled draw mode — render only a portion of a sprite based on a fill ratio along a configurable axis/origin. Useful for progress bars, cooldown indicators, etc.
Changes
FilledSpriteAssembler(~613 lines) handles vertex generation for filled modeSpriteFilledMode— direction (Horizontal / Vertical / Radial90 / Radial180 / Radial360)SpriteFilledOrigin— anchor point for the fill directionSpriteDrawModewithFilledISpriteRendererinterfaceSpriteRendererintegrates filled assemblerTest Plan
SpriteFilledMode×SpriteFilledOrigincombinationsSummary by CodeRabbit
Release Notes