feat(2d): support rotated sprites in atlas#2990
Conversation
WalkthroughThe PR expands sprite UV handling to support a full 16-vertex (4×4) grid with atlas 90° rotation awareness. Sprite's UV storage and computation are rewritten; size calculation accounts for rotated atlas dimensions; the loader explicitly initializes the rotation flag; and three assemblers adapt their UV reading to use column-major indexing into the new grid. ChangesUV Grid Expansion and Atlas Rotation Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/sprite/Sprite.ts`:
- Around line 299-310: The sprite's cached size and UVs must be invalidated when
_atlasRotated changes: in the atlasRotated setter (the code that flips
this._atlasRotated), after toggling the boolean clear the cached computed size
and UVs by setting this._automaticWidth and this._automaticHeight to undefined
(or null) and force UV recompute/clear by invoking or resetting whatever
_getUVs() cache (e.g., call this._getUVs() or set the UV cache to null) and mark
the sprite dirty so width/height/_getUVs() will be recalculated; apply the same
invalidation logic wherever _atlasRotated can change.
- Around line 352-366: The rotated branch misapplies trim offsets—when
atlasRotated is true you must rotate the offsets and border axes to match the
packed axes mapping (original region/offset left/top/right/bottom →
bottom/left/top/right); update the computations that set left/top/right/bottom
and bLeft/bTop/bRight/bBottom to use the swapped offsets and border components:
for the X-side math (calculations that use atlasRegionW/realWidth and
regionBottom/regionTop) use offsetBottom/offsetTop and border.y/w where
appropriate, and for the Y-side math (calculations that use
atlasRegionH/realHeight and regionLeft/regionRight) use offsetLeft/offsetRight
and border.x/z accordingly so trimmed rotated sprites and 9-slice boundaries
align correctly (references: atlasRotated, realWidth, realHeight,
atlasRegionX/Y/W/H, regionLeft/Top/Right/Bottom, offsetLeft/Top/Right/Bottom,
border.x/y/w/z, regionW/regionH).
🪄 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: c7a666d6-7c92-41c5-b8a9-6232f629a763
📒 Files selected for processing (5)
packages/core/src/2d/assembler/SimpleSpriteAssembler.tspackages/core/src/2d/assembler/SlicedSpriteAssembler.tspackages/core/src/2d/assembler/TiledSpriteAssembler.tspackages/core/src/2d/sprite/Sprite.tspackages/loader/src/SpriteAtlasLoader.ts
| const { _texture, _atlasRegion, _atlasRegionOffset, _region, _atlasRotated } = this; | ||
| const ppuReciprocal = 1.0 / Engine._pixelsPerUnit; | ||
| // 先算 atlas 中绝对像素（texture 不一定是方形，必须各自乘对应维度） | ||
| const atlasPxW = _texture.width * _atlasRegion.width; | ||
| const atlasPxH = _texture.height * _atlasRegion.height; | ||
| // atlas 顺时针 pack 90°：原图 W×H 在 atlas 中占 H×W 区域，仅交换 atlasPx 的 W/H | ||
| const originWidth = _atlasRotated ? atlasPxH : atlasPxW; | ||
| const originHeight = _atlasRotated ? atlasPxW : atlasPxH; | ||
| this._automaticWidth = | ||
| ((_texture.width * _atlasRegion.width) / (1 - _atlasRegionOffset.x - _atlasRegionOffset.z)) * | ||
| _region.width * | ||
| pixelsPerUnitReciprocal; | ||
| (originWidth / (1 - _atlasRegionOffset.x - _atlasRegionOffset.z)) * _region.width * ppuReciprocal; | ||
| this._automaticHeight = | ||
| ((_texture.height * _atlasRegion.height) / (1 - _atlasRegionOffset.y - _atlasRegionOffset.w)) * | ||
| _region.height * | ||
| pixelsPerUnitReciprocal; | ||
| (originHeight / (1 - _atlasRegionOffset.y - _atlasRegionOffset.w)) * _region.height * ppuReciprocal; |
There was a problem hiding this comment.
Invalidate cached size and UVs when atlasRotated changes.
These paths now depend on _atlasRotated, but the setter at Lines 121-124 still only flips the boolean. If width, height, or _getUVs() has already been evaluated, changing sprite.atlasRotated leaves stale cached results until some other property dirties the sprite.
Suggested fix
set atlasRotated(value: boolean) {
- if (this._atlasRotated != value) {
+ if (this._atlasRotated !== value) {
this._atlasRotated = value;
+ this._dispatchSpriteChange(SpriteModifyFlags.atlasRegion);
+ if (this._customWidth === undefined || this._customHeight === undefined) {
+ this._dispatchSpriteChange(SpriteModifyFlags.size);
+ }
}
}Also applies to: 345-392
🤖 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/Sprite.ts` around lines 299 - 310, The sprite's
cached size and UVs must be invalidated when _atlasRotated changes: in the
atlasRotated setter (the code that flips this._atlasRotated), after toggling the
boolean clear the cached computed size and UVs by setting this._automaticWidth
and this._automaticHeight to undefined (or null) and force UV recompute/clear by
invoking or resetting whatever _getUVs() cache (e.g., call this._getUVs() or set
the UV cache to null) and mark the sprite dirty so width/height/_getUVs() will
be recalculated; apply the same invalidation logic wherever _atlasRotated can
change.
| const realWidth = atlasRegionW / (1 - offsetLeft - offsetRight); | ||
| const realHeight = atlasRegionH / (1 - offsetTop - offsetBottom); | ||
| // Coordinates of the four boundaries. | ||
| const left = Math.max(regionX - offsetLeft, 0) * realWidth + atlasRegionX; | ||
| const top = Math.max(regionBottom - offsetTop, 0) * realHeight + atlasRegionY; | ||
| const right = atlasRegionW + atlasRegionX - Math.max(regionRight - offsetRight, 0) * realWidth; | ||
| const bottom = atlasRegionH + atlasRegionY - Math.max(regionY - offsetBottom, 0) * realHeight; | ||
| const { x: borderLeft, y: borderBottom, z: borderRight, w: borderTop } = this._border; | ||
| // Left-Bottom | ||
| uv[0].set(left, bottom); | ||
| // Border ( Left-Bottom ) | ||
| uv[1].set( | ||
| (regionX - offsetLeft + borderLeft * regionW) * realWidth + atlasRegionX, | ||
| atlasRegionH + atlasRegionY - (regionY - offsetBottom + borderBottom * regionH) * realHeight | ||
| ); | ||
| // Border ( Right-Top ) | ||
| uv[2].set( | ||
| atlasRegionW + atlasRegionX - (regionRight - offsetRight + borderRight * regionW) * realWidth, | ||
| (regionBottom - offsetTop + borderTop * regionH) * realHeight + atlasRegionY | ||
| ); | ||
| // Right-Top | ||
| uv[3].set(right, top); | ||
| // 4 个外边界 + 4 个 9-slice 内边界 | ||
| let left: number, top: number, right: number, bottom: number; | ||
| let bLeft: number, bTop: number, bRight: number, bBottom: number; | ||
| if (atlasRotated) { | ||
| // 原图 region/offset (left/top/right/bottom) 在 atlas 中映射为 (bottom/left/top/right) | ||
| left = Math.max(regionBottom - offsetLeft, 0) * realWidth + atlasRegionX; | ||
| top = Math.max(regionLeft - offsetTop, 0) * realHeight + atlasRegionY; | ||
| right = atlasRegionW + atlasRegionX - Math.max(regionTop - offsetRight, 0) * realWidth; | ||
| bottom = atlasRegionH + atlasRegionY - Math.max(regionRight - offsetBottom, 0) * realHeight; | ||
| bLeft = (regionBottom - offsetLeft + border.y * regionH) * realWidth + atlasRegionX; | ||
| bTop = (regionLeft - offsetTop + border.x * regionW) * realHeight + atlasRegionY; | ||
| bRight = atlasRegionW + atlasRegionX - (regionTop - offsetRight + border.w * regionH) * realWidth; | ||
| bBottom = atlasRegionH + atlasRegionY - (regionRight - offsetBottom + border.z * regionW) * realHeight; |
There was a problem hiding this comment.
Rotate the trim offsets with the packed axes.
In the rotated branch, atlas-X is derived from the sprite’s vertical span and atlas-Y from the horizontal span, but the code still feeds offsetLeft/offsetRight into the X-side math and offsetTop/offsetBottom into the Y-side math. That breaks trimmed rotated sprites when horizontal and vertical trims differ, and the 9-slice boundaries drift with them.
Suggested fix
- const realWidth = atlasRegionW / (1 - offsetLeft - offsetRight);
- const realHeight = atlasRegionH / (1 - offsetTop - offsetBottom);
+ const realWidth = atlasRotated
+ ? atlasRegionW / (1 - offsetTop - offsetBottom)
+ : atlasRegionW / (1 - offsetLeft - offsetRight);
+ const realHeight = atlasRotated
+ ? atlasRegionH / (1 - offsetLeft - offsetRight)
+ : atlasRegionH / (1 - offsetTop - offsetBottom);
@@
if (atlasRotated) {
- left = Math.max(regionBottom - offsetLeft, 0) * realWidth + atlasRegionX;
- top = Math.max(regionLeft - offsetTop, 0) * realHeight + atlasRegionY;
- right = atlasRegionW + atlasRegionX - Math.max(regionTop - offsetRight, 0) * realWidth;
- bottom = atlasRegionH + atlasRegionY - Math.max(regionRight - offsetBottom, 0) * realHeight;
- bLeft = (regionBottom - offsetLeft + border.y * regionH) * realWidth + atlasRegionX;
- bTop = (regionLeft - offsetTop + border.x * regionW) * realHeight + atlasRegionY;
- bRight = atlasRegionW + atlasRegionX - (regionTop - offsetRight + border.w * regionH) * realWidth;
- bBottom = atlasRegionH + atlasRegionY - (regionRight - offsetBottom + border.z * regionW) * realHeight;
+ left = Math.max(regionBottom - offsetBottom, 0) * realWidth + atlasRegionX;
+ top = Math.max(regionLeft - offsetLeft, 0) * realHeight + atlasRegionY;
+ right = atlasRegionW + atlasRegionX - Math.max(regionTop - offsetTop, 0) * realWidth;
+ bottom = atlasRegionH + atlasRegionY - Math.max(regionRight - offsetRight, 0) * realHeight;
+ bLeft = (regionBottom - offsetBottom + border.y * regionH) * realWidth + atlasRegionX;
+ bTop = (regionLeft - offsetLeft + border.x * regionW) * realHeight + atlasRegionY;
+ bRight = atlasRegionW + atlasRegionX - (regionTop - offsetTop + border.w * regionH) * realWidth;
+ bBottom = atlasRegionH + atlasRegionY - (regionRight - offsetRight + border.z * regionW) * realHeight;
} else {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const realWidth = atlasRegionW / (1 - offsetLeft - offsetRight); | |
| const realHeight = atlasRegionH / (1 - offsetTop - offsetBottom); | |
| // Coordinates of the four boundaries. | |
| const left = Math.max(regionX - offsetLeft, 0) * realWidth + atlasRegionX; | |
| const top = Math.max(regionBottom - offsetTop, 0) * realHeight + atlasRegionY; | |
| const right = atlasRegionW + atlasRegionX - Math.max(regionRight - offsetRight, 0) * realWidth; | |
| const bottom = atlasRegionH + atlasRegionY - Math.max(regionY - offsetBottom, 0) * realHeight; | |
| const { x: borderLeft, y: borderBottom, z: borderRight, w: borderTop } = this._border; | |
| // Left-Bottom | |
| uv[0].set(left, bottom); | |
| // Border ( Left-Bottom ) | |
| uv[1].set( | |
| (regionX - offsetLeft + borderLeft * regionW) * realWidth + atlasRegionX, | |
| atlasRegionH + atlasRegionY - (regionY - offsetBottom + borderBottom * regionH) * realHeight | |
| ); | |
| // Border ( Right-Top ) | |
| uv[2].set( | |
| atlasRegionW + atlasRegionX - (regionRight - offsetRight + borderRight * regionW) * realWidth, | |
| (regionBottom - offsetTop + borderTop * regionH) * realHeight + atlasRegionY | |
| ); | |
| // Right-Top | |
| uv[3].set(right, top); | |
| // 4 个外边界 + 4 个 9-slice 内边界 | |
| let left: number, top: number, right: number, bottom: number; | |
| let bLeft: number, bTop: number, bRight: number, bBottom: number; | |
| if (atlasRotated) { | |
| // 原图 region/offset (left/top/right/bottom) 在 atlas 中映射为 (bottom/left/top/right) | |
| left = Math.max(regionBottom - offsetLeft, 0) * realWidth + atlasRegionX; | |
| top = Math.max(regionLeft - offsetTop, 0) * realHeight + atlasRegionY; | |
| right = atlasRegionW + atlasRegionX - Math.max(regionTop - offsetRight, 0) * realWidth; | |
| bottom = atlasRegionH + atlasRegionY - Math.max(regionRight - offsetBottom, 0) * realHeight; | |
| bLeft = (regionBottom - offsetLeft + border.y * regionH) * realWidth + atlasRegionX; | |
| bTop = (regionLeft - offsetTop + border.x * regionW) * realHeight + atlasRegionY; | |
| bRight = atlasRegionW + atlasRegionX - (regionTop - offsetRight + border.w * regionH) * realWidth; | |
| bBottom = atlasRegionH + atlasRegionY - (regionRight - offsetBottom + border.z * regionW) * realHeight; | |
| const realWidth = atlasRotated | |
| ? atlasRegionW / (1 - offsetTop - offsetBottom) | |
| : atlasRegionW / (1 - offsetLeft - offsetRight); | |
| const realHeight = atlasRotated | |
| ? atlasRegionH / (1 - offsetLeft - offsetRight) | |
| : atlasRegionH / (1 - offsetTop - offsetBottom); | |
| // 4 个外边界 + 4 个 9-slice 内边界 | |
| let left: number, top: number, right: number, bottom: number; | |
| let bLeft: number, bTop: number, bRight: number, bBottom: number; | |
| if (atlasRotated) { | |
| // 原图 region/offset (left/top/right/bottom) 在 atlas 中映射为 (bottom/left/top/right) | |
| left = Math.max(regionBottom - offsetBottom, 0) * realWidth + atlasRegionX; | |
| top = Math.max(regionLeft - offsetLeft, 0) * realHeight + atlasRegionY; | |
| right = atlasRegionW + atlasRegionX - Math.max(regionTop - offsetTop, 0) * realWidth; | |
| bottom = atlasRegionH + atlasRegionY - Math.max(regionRight - offsetRight, 0) * realHeight; | |
| bLeft = (regionBottom - offsetBottom + border.y * regionH) * realWidth + atlasRegionX; | |
| bTop = (regionLeft - offsetLeft + border.x * regionW) * realHeight + atlasRegionY; | |
| bRight = atlasRegionW + atlasRegionX - (regionTop - offsetTop + border.w * regionH) * realWidth; | |
| bBottom = atlasRegionH + atlasRegionY - (regionRight - offsetRight + border.z * regionW) * realHeight; |
🤖 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/Sprite.ts` around lines 352 - 366, The rotated
branch misapplies trim offsets—when atlasRotated is true you must rotate the
offsets and border axes to match the packed axes mapping (original region/offset
left/top/right/bottom → bottom/left/top/right); update the computations that set
left/top/right/bottom and bLeft/bTop/bRight/bBottom to use the swapped offsets
and border components: for the X-side math (calculations that use
atlasRegionW/realWidth and regionBottom/regionTop) use offsetBottom/offsetTop
and border.y/w where appropriate, and for the Y-side math (calculations that use
atlasRegionH/realHeight and regionLeft/regionRight) use offsetLeft/offsetRight
and border.x/z accordingly so trimmed rotated sprites and 9-slice boundaries
align correctly (references: atlasRotated, realWidth, realHeight,
atlasRegionX/Y/W/H, regionLeft/Top/Right/Bottom, offsetLeft/Top/Right/Bottom,
border.x/y/w/z, regionW/regionH).
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.
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.
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.
总结
支持 atlas 中旋转 90° pack 的 sprite。UV 网格从 4 点扩到 16 点，旋转完全烘焙进 _updateUVs 的两个 if (atlasRotated) 分支，三个 assembler 保持 rotation-agnostic（无 per-vertex 分支，热路径无回归）。_calDefaultSize 的 W/H swap 正确。方向合理，但 setter 漏脏标记这个 P1 仍未修，且有一处旋转+裁剪的边界需要测试坐实。
问题
P1
-
Sprite.ts:121-125—atlasRotatedsetter 不触发 UV/Size 脏标记set atlasRotated(value: boolean) { if (this._atlasRotated != value) { this._atlasRotated = value; // 只翻 bool，啥都不 dispatch } }
_updateUVs（受SpriteUpdateFlags.uvs门控）和_calDefaultSize（受automaticSize门控）现在都读_atlasRotated，但运行时sprite.atlasRotated = x后这俩脏标记都不会置位 → UV/size 保持陈旧，直到别的改动恰好把 sprite 弄脏。修复模式就在下方 13 行的atlasRegion/atlasRegionOffsetsetter 里现成：set atlasRotated(value: boolean) { if (this._atlasRotated !== value) { this._atlasRotated = value; this._dispatchSpriteChange(SpriteModifyFlags.atlasRegion); // 触发 uvs // 若 width/height 非自定义，同 atlasRegion setter 那样再 dispatch size } }
顺带
!=改!==。SpriteAtlasLoader.ts:105那行sprite.atlasRotated = config.atlasRotated ?? false也因此 bug 而无法在二次复用时正确触发——同根因。
P2（需要一个测试来定性，不是断言它一定错）
-
Sprite.ts:352-366— 旋转分支里 trim offset 的轴向配对存疑，缺非对称裁剪的 rotated 用例realWidth = atlasRegionW / (1 - offsetLeft - offsetRight)：atlasRegionW是 atlas-X 方向的 packed 跨度（旋转后对应原图竖直方向），却除以水平 trim（offsetLeft/offsetRight）。旋转分支里left = (regionBottom - offsetLeft) * realWidth + atlasRegionX又把竖直 region 分数regionBottom和水平 trimoffsetLeft配在一起。我之前几轮把这点判为"公式正确"并关闭了，但重新推导发现：是否正确取决于编辑器/打包器导出 offset 用的是原图坐标系还是 atlas 坐标系——
SpriteAtlasLoader.ts:103把offsetLeft/Right按invW、offsetTop/Bottom按invH归一化，暗示 offset 存的是 atlas 帧。这个只在旋转 + H/V 裁剪不对称的 sprite 上才显形，当前测试没有覆盖。请补一个atlasRotated=true且offsetLeft ≠ offsetTop、offsetRight ≠ offsetBottom的用例（断言四角 UV 落在期望 atlas 坐标），由它来定性而非靠推导。如果该用例通过，这条即关闭；不通过则按 CodeRabbit 建议交换轴向。
自检
setter 漏脏标记已对照 HEAD 0a0c4a72 实际源码确认（与下方 atlasRegion setter 对比）。trim-offset 一条我没有断言它是 bug——重新推导后发现依赖打包器约定无法纯静态确认，故降为"需测试坐实"的 P2，并附上 loader 归一化作为线索。assembler 无 per-vertex 旋转分支、热路径无回归已确认。
2 participants
Summary
Adds support for rotated sprites in
SpriteAtlas— TexturePacker-style atlases that pack sprites with 90° rotation to optimize space.Changes
Sprite.ts— extends UV calculation to handle rotated source rect (~97 lines updated)SimpleSpriteAssembler/SlicedSpriteAssembler/TiledSpriteAssembler— vertex/UV generation respects rotation flagSpriteAtlasLoader.ts— propagates rotation flag from atlas metadata to SpriteTest Plan
Summary by CodeRabbit
Bug Fixes
Refactor