Upgrade to pixi v8 #2728
Upgrade to pixi v8 #2728
Conversation
|
QA Wolf here! As you write new code it's important that your test coverage is keeping up. |
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## qa #2728 +/- ##
=======================================
Coverage 91.06% 91.06%
=======================================
Files 401 401
Lines 93279 93279
=======================================
Hits 84949 84949
Misses 8330 8330 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| }; | ||
|
|
||
| @vertex | ||
| fn mainVertex(@location(0) aPosition : vec2<f32>, @location(1) aUV : vec2<f32>, ) -> VertexOutput { |
There was a problem hiding this comment.
There's a syntax error in the WGSL shader code - the parameter list for the mainVertex function has a trailing comma after the last parameter that should be removed:
fn mainVertex(@location(0) aPosition : vec2<f32>, @location(1) aUV : vec2<f32>, ) -> VertexOutput {Should be:
fn mainVertex(@location(0) aPosition : vec2<f32>, @location(1) aUV : vec2<f32>) -> VertexOutput {This trailing comma could cause shader compilation errors in WebGPU implementations.
| fn mainVertex(@location(0) aPosition : vec2<f32>, @location(1) aUV : vec2<f32>, ) -> VertexOutput { | |
| fn mainVertex(@location(0) aPosition : vec2<f32>, @location(1) aUV : vec2<f32>) -> VertexOutput { |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
| }); | ||
| } | ||
| export class CellsDrawRects extends Graphics { | ||
| update(drawRects: DrawRects[]) {} |
There was a problem hiding this comment.
The update method is empty but should implement drawing rectangles based on the drawRects parameter. Without implementation, text decorations like strikethrough won't be rendered. Consider implementing the drawing logic similar to how it was done in the previous version, but adapted for PixiJS v8's new graphics API.
| update(drawRects: DrawRects[]) {} | |
| update(drawRects: DrawRects[]) { | |
| this.clear(); | |
| if (!drawRects.length) return; | |
| for (const rect of drawRects) { | |
| this.beginFill(rect.color); | |
| this.drawRect(rect.x, rect.y, rect.width, rect.height); | |
| this.endFill(); | |
| } | |
| } |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
| } | ||
|
|
||
| // Return the final color with alpha | ||
| return vec4<f32>(0.0, 0.0, 0.0, alpha); |
There was a problem hiding this comment.
In the WGSL fragment shader in cellLabelShaderTint.ts, the function returns a hardcoded black color with alpha:
return vec4<f32>(0.0, 0.0, 0.0, alpha);This should use the vertex color input parameter instead, matching the behavior in the main shader. Consider changing to:
return vec4<f32>(vColors.rgb, alpha);This ensures text colors are properly applied when using the tinted shader variant.
| return vec4<f32>(0.0, 0.0, 0.0, alpha); | |
| return vec4<f32>(vColors.rgb, alpha); |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
Upgrade pixi to v8