Skip to content

fix: flipY in webgpu renderpass#211

Merged
xiaoiver merged 1 commit intoreleasefrom
fix-flipy
Apr 17, 2026
Merged

fix: flipY in webgpu renderpass#211
xiaoiver merged 1 commit intoreleasefrom
fix-flipy

Conversation

@xiaoiver
Copy link
Copy Markdown
Contributor

@xiaoiver xiaoiver commented Apr 17, 2026

🤔 This is a

  • New feature
  • Bug fix
  • Site / Document optimization
  • TypeScript definition update
  • Refactoring
  • Performance improvement
  • Code style optimization
  • Test Case
  • Branch merge
  • Other (about what?)

🔗 Related issue link

💡 Background and solution

📝 Changelog

Language Changelog
🇺🇸 English
🇨🇳 Chinese

☑️ Self Check before Merge

  • Doc is updated/provided or not needed
  • Demo is updated/provided or not needed
  • TypeScript definition is updated/provided or not needed
  • Changelog is provided or not needed

@xiaoiver xiaoiver merged commit 7bf8ff6 into release Apr 17, 2026
0 of 6 checks passed
@xiaoiver xiaoiver deleted the fix-flipy branch April 17, 2026 09:24
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request implements _viewportFlipHeight to use the active render target height for Y-flipping, ensuring correct rendering for offscreen passes. Feedback suggests simplifying the attachment level logic, avoiding string-based access to private members, and removing redundant nullish coalescing.

Comment thread src/webgpu/RenderPass.ts
Comment on lines +214 to +217
const level0 =
this.gfxColorAttachmentLevel.length > 0
? this.gfxColorAttachmentLevel[0] || 0
: 0;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The logic for determining level0 can be simplified. In TypeScript, accessing an index beyond the array length returns undefined, and undefined || 0 correctly evaluates to 0. This avoids the need for an explicit length check.

Suggested change
const level0 =
this.gfxColorAttachmentLevel.length > 0
? this.gfxColorAttachmentLevel[0] || 0
: 0;
const level0 = this.gfxColorAttachmentLevel[0] || 0;

Comment thread src/webgpu/RenderPass.ts
const ds = descriptor.depthStencilAttachment as unknown as Attachment_WebGPU;
this._viewportFlipHeight = Math.max(1, ds.height >>> 0);
} else {
this._viewportFlipHeight = this.device['swapChainHeight'];
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Accessing the private member swapChainHeight via string indexing (this.device['swapChainHeight']) is a hack that bypasses TypeScript's visibility checks. This makes the code fragile and dependent on internal implementation details of the Device_WebGPU class. It is recommended to expose this property through a public getter or a dedicated method in the Device_WebGPU class.

Comment thread src/webgpu/RenderPass.ts
private flipY(y: number, h: number) {
const height = this.gfxColorAttachment[0].height;
const height =
this._viewportFlipHeight ?? this.device['swapChainHeight'];
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Avoid using string indexing to access private members. Additionally, since _viewportFlipHeight is initialized in all branches of the setRenderPassDescriptor method (which is called at the start of every render pass), this nullish coalescing fallback is likely redundant.

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.

1 participant