feat: generate msdf and render msdf #17
Conversation
WalkthroughThis update introduces a full pipeline for generating, importing, and rendering multi-channel signed distance field (MSDF) icons using WebGPU. It adds scripts for MSDF font generation, new WebGPU shader programs and pipelines for MSDF rendering, icon asset management in Zig, and integration of icon textures and metadata into the rendering flow. Supporting utilities and TypeScript/Zig type declarations are also updated. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Script as generate-msdf.js
participant Assets as SVG/Font Files
participant App as Application Startup
participant GPU as WebGPU
participant Zig as Zig Logic
Dev->>Script: Run npm run generate-msdf
Script->>Assets: Read SVG icons
Script->>Assets: Generate SVG font, TTF, PNG, JSON
Script->>Assets: Output MSDF assets
App->>Assets: Import icons.png and icons.json
App->>GPU: Upload icons.png as GPU texture
App->>Zig: Call import_icons(flattened metadata)
Zig->>GPU: On render, call draw_msdf(vertexData, texture)
GPU->>GPU: Use drawMSDF pipeline and shader
GPU->>GPU: Render icons using MSDF texture and metadata
Possibly related PRs
Suggested labels
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🔭 Outside diff range comments (1)
src/suspended/rendering vectors/generateMSDF/shader.wgsl (1)
311-312: Incomplete implementation.The file ends with an incomplete comment about color calculation, suggesting the implementation may be unfinished.
The shader appears to have an incomplete implementation. Please complete the color calculation logic or remove the trailing comment.
♻️ Duplicate comments (6)
src/suspended/rendering vectors/generateMSDF/shader_gemini_better.wgsl (1)
170-234: Extract common distance calculation logic.This function duplicates the distance calculation logic from
signedPseudoDistance. The same issues apply (zero curve length and redundant normalize operations).Consider extracting the common distance calculation into a shared function to avoid code duplication and ensure consistent behavior.
src/suspended/rendering vectors/generateMSDF/shader_gemini.wgsl (3)
223-227: Debug visualization is enabled.Similar to other shaders, individual channel visualization is enabled instead of the median MSDF.
Switch to median-based output for production use.
55-55: Add protection against zero curve length.Same issue as in other shader files - potential division by zero if curve_length is 0.
- let samples = u32(curve_length); + let samples = max(u32(curve_length), 16u);
82-82: Remove redundant normalize operations.Same redundant operations as in other shader files.
The expression
normalize(tangentAtStart) * length(tangentAtStart)should be simplified to justtangentAtStart. Apply to lines 82, 87, 96, and 101.src/suspended/rendering vectors/generateMSDF/our_shader.wgsl (2)
118-118: Add protection against zero curve length.Same division by zero risk as in other shader files.
- let samples = u32(curve_length); + let samples = max(u32(curve_length), 16u);
145-145: Remove redundant normalize operations.Same redundant operations found in other shader files.
Simplify
normalize(tangentAtStart) * length(tangentAtStart)to justtangentAtStarton lines 145, 150, 159, and 164.
🧹 Nitpick comments (15)
src/index.ts (1)
92-107: Consider adding error handling for image loading.The icon loading logic is correct, with proper texture creation and data normalization. However, consider adding error handling for the image loading process.
const icons = new Image() icons.src = IconsPng icons.onload = () => { textures[0].texture = createTextureFromSource(device, icons, { flipY: true }) import_icons( IconsJson.chars.flatMap((char) => [ char.id, char.x / icons.width, char.y / icons.height, char.width / icons.width, char.height / icons.height, char.width, char.height, ]) ) } + icons.onerror = (error) => { + console.error('Failed to load icons:', error) + }src/suspended/rendering vectors/index.ts (1)
17-31: Remove unnecessary type assertions.The type assertions for
Pointon lines 25-28 suggest that the segment structure might not be properly typed.If the segments are properly typed as containing 4 points for cubic beziers, the assertions shouldn't be necessary:
segments.forEach(({ points: [start, cp1, cp2, end] }, i) => { ctx.strokeStyle = `rgb(0, ${(i / segments.length) * 255}, 0)` ctx.lineWidth = 10 ctx.beginPath() ctx.moveTo(start.x, start.y) ctx.bezierCurveTo( cp1.x, cp1.y, - (cp2 as Point).x, - (cp2 as Point).y, - (end as Point).x, - (end as Point).y + cp2.x, + cp2.y, + end.x, + end.y ) ctx.stroke() })msdf/generate-msdf.js (1)
53-66: Make icon configuration more maintainable.The icon paths and unicode values are hardcoded, making it difficult to add new icons.
Consider creating a configuration array:
+const iconConfig = [ + { path: 'icons/rotate.svg', unicode: '\uE001', name: 'rotate' }, + { path: 'icons/trash-bin.svg', unicode: '\uE002', name: 'trash-bin' } +] + -const rotateIcon = fs.createReadStream(getPath('icons/rotate.svg')) -rotateIcon.metadata = { - unicode: ['\uE001'], - name: 'rotate', -} -fontStream.write(rotateIcon) - -const trashIcon = fs.createReadStream(getPath('icons/trash-bin.svg')) -trashIcon.metadata = { - unicode: ['\uE002'], - name: 'trash-bin', -} -fontStream.write(trashIcon) +iconConfig.forEach(({ path, unicode, name }) => { + const iconStream = fs.createReadStream(getPath(path)) + iconStream.metadata = { + unicode: [unicode], + name: name, + } + fontStream.write(iconStream) +})src/suspended/rendering vectors/svgToSegments.ts (2)
232-293: Remove unused code.The
EdgeColorenum, color-related functions, anddirectionfunction are not used anywhere in this file.Remove the unused code (lines 232-293) to improve maintainability. If this code is intended for future use, consider moving it to a separate module or adding a TODO comment explaining its purpose.
222-225: Consider implementing quadratic Bezier and arc support.The comment indicates missing support for common SVG commands.
Would you like me to help implement support for quadratic Bezier curves (Q, T commands) and arcs (A command)? I can open a new issue to track this enhancement.
src/logic/index.zig (1)
282-295: Remove commented-out code.Large blocks of commented code should be removed to improve code maintainability.
- // std.debug.print("icon id: {}", data.len); - // var iterator = data.iterator(); - // while (iterator.next()) |icon| { - // std.debug.print("icon id: {}", icon.id); - // } - - // Notify about the new assets - // var result = std.heap.page_allocator.alloc(AssetZig, state.assets.count()) catch unreachable; - // var i: usize = 0; - // for (state.assets.items(), 0..) |entry| { - // result[i] = entry.value_ptr.serialize(); - // i += 1; - // } - // on_asset_update_cb(result);src/WebGPU/programs/drawMSDF/getProgram.ts (2)
93-93: Implement bind group caching for better performance.The comment indicates that bind groups should be pre-created and reused rather than created on every draw call, which is a valid performance concern.
Would you like me to implement a bind group caching mechanism to improve rendering performance?
79-79: Rename function to match MSDF context.The function is named
drawTexturebut it's specifically for MSDF rendering. Consider renaming for clarity.- return function drawTexture( + return function drawMSDF(src/suspended/rendering vectors/generateMSDF/shader_bette_RbetteR_gemini.wgsl (2)
1-342: Consider the implications of using experimental legacy code.This shader is in the suspended directory and contains a comment indicating it's marked as "legacy" by the original author. Additionally, it's extremely complex (342 lines) which may have performance implications.
Key concerns:
- The code implements multiple similar functions (
signedPseudoDistanceandpseudoDistance)- Complex winding number calculations that may be computationally expensive
- The author has marked this approach as legacy (line 343)
Given that this is experimental legacy code, consider whether a simpler, more modern MSDF generation approach would be more appropriate. Have you evaluated the performance impact of this implementation?
43-168: Remove code duplication between distance calculation functions.The
signedPseudoDistanceandpseudoDistancefunctions share significant logic for Bezier evaluation and distance calculation. Consider refactoring to eliminate duplication.Extract the common Bezier evaluation and closest point finding logic into shared helper functions to reduce code duplication and improve maintainability.
Also applies to: 193-229
src/suspended/rendering vectors/generateMSDF/shader_gemini_better.wgsl (2)
236-288: Consider using shared Bezier evaluation function.The function duplicates Bezier evaluation logic that could be extracted into a shared
evalBezierfunction (similar to other shader files).Define and use a shared cubic Bezier evaluation function to reduce code duplication and improve maintainability.
340-344: Debug visualization code is enabled.The shader is currently outputting individual channel distances for debugging rather than the median MSDF value.
For production use, consider using the median-based output:
- // let color = vec3f(finalSDF * 0.1 + 0.5); - - // For debugging, you can visualize the individual channels: - let color = vec3f(signedRed, signedGreen, signedBlue) * 0.1 + 0.5; + let color = vec3f(finalSDF * 0.1 + 0.5); + + // For debugging, you can visualize the individual channels: + // let color = vec3f(signedRed, signedGreen, signedBlue) * 0.1 + 0.5;src/suspended/rendering vectors/generateMSDF/shader_gemini_freedom_solution.wgsl (1)
165-165: Consider making the normalization range configurable.The hardcoded value of 20.0 for range normalization may not be suitable for all scales of shapes.
Consider passing the range as a uniform parameter or calculating it based on the shape bounds.
src/suspended/rendering vectors/generateMSDF/our_shader.wgsl (2)
265-265: Remove double semicolon.Syntax issue with double semicolon.
- let curve_samples = u32(curve_length);; + let curve_samples = u32(curve_length);
345-367: Clean up commented debug code.Large block of commented code makes the implementation harder to read.
Consider removing the commented debug code or moving it to a separate debug shader variant.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
msdf/icons/rotate.svgis excluded by!**/*.svgmsdf/icons/trash-bin.svgis excluded by!**/*.svgpackage-lock.jsonis excluded by!**/package-lock.jsonsrc/suspended/rendering vectors/Screenshot 2025-07-02 at 5.54.20 PM.pngis excluded by!**/*.png
📒 Files selected for processing (25)
.gitignore(1 hunks)msdf/generate-msdf.js(1 hunks)package.json(2 hunks)src/WebGPU/programs/draw/getProgram.ts(1 hunks)src/WebGPU/programs/draw/shader.wgsl(1 hunks)src/WebGPU/programs/drawMSDF/getProgram.ts(1 hunks)src/WebGPU/programs/drawMSDF/shader.wgsl(1 hunks)src/WebGPU/programs/initPrograms.ts(3 hunks)src/index.ts(3 hunks)src/logic/index.d.ts(3 hunks)src/logic/index.zig(6 hunks)src/logic/types.zig(1 hunks)src/run.ts(2 hunks)src/suspended/rendering vectors/generateMSDF/getProgram.ts(1 hunks)src/suspended/rendering vectors/generateMSDF/our_shader.wgsl(1 hunks)src/suspended/rendering vectors/generateMSDF/shader.wgsl(1 hunks)src/suspended/rendering vectors/generateMSDF/shader_bette_RbetteR_gemini.wgsl(1 hunks)src/suspended/rendering vectors/generateMSDF/shader_gemini.wgsl(1 hunks)src/suspended/rendering vectors/generateMSDF/shader_gemini_better.wgsl(1 hunks)src/suspended/rendering vectors/generateMSDF/shader_gemini_freedom_solution.wgsl(1 hunks)src/suspended/rendering vectors/index.ts(1 hunks)src/suspended/rendering vectors/svgToSegments.ts(1 hunks)src/types.ts(1 hunks)src/utils/getSvgSource.ts(1 hunks)tsconfig.json(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (7)
src/run.ts (1)
src/WebGPU/programs/initPrograms.ts (1)
drawMSDF(21-21)
src/suspended/rendering vectors/index.ts (2)
src/suspended/rendering vectors/svgToSegments.ts (1)
svgToSegments(295-390)src/types.ts (1)
Point(1-4)
src/logic/index.d.ts (1)
src/types.ts (1)
Point(1-4)
src/suspended/rendering vectors/svgToSegments.ts (1)
src/types.ts (4)
Point(1-4)Segment(9-12)Line(6-6)CubicBezier(8-8)
src/WebGPU/programs/draw/getProgram.ts (3)
src/WebGPU/programs/drawMSDF/getProgram.ts (1)
getProgram(5-114)src/suspended/rendering vectors/generateMSDF/getProgram.ts (1)
getProgram(8-97)src/WebGPU/programs/initPrograms.ts (1)
drawTriangle(12-12)
src/index.ts (2)
src/WebGPU/getTexture/index.ts (1)
createTextureFromSource(106-117)src/logic/index.d.ts (1)
import_icons(42-42)
src/WebGPU/programs/drawMSDF/getProgram.ts (3)
src/WebGPU/programs/draw/getProgram.ts (1)
getProgram(6-101)src/suspended/rendering vectors/generateMSDF/getProgram.ts (1)
getProgram(8-97)src/WebGPU/programs/initPrograms.ts (1)
drawTexture(18-18)
🔇 Additional comments (33)
.gitignore (1)
18-18: LGTM! Appropriate addition for generated files.Adding the MSDF output directory to
.gitignoreis the correct approach for excluding generated artifacts from version control.src/WebGPU/programs/draw/shader.wgsl (1)
1-27: Well-structured WGSL shader for colored vertex rendering.The shader implementation follows WebGPU best practices:
- Proper vertex attribute locations and uniform bindings
- Correct matrix transformation pipeline
- Clean data flow from vertex to fragment shader
tsconfig.json (1)
19-21: Appropriate exclusion of experimental code.Adding the
excludefield to prevent TypeScript compilation of the suspended directory is a good approach for keeping experimental code separate from the main build.src/WebGPU/programs/initPrograms.ts (3)
10-10: Consistent import pattern.The import follows the established pattern for other GPU programs.
21-21: Proper type declaration.The export declaration correctly uses
ReturnTypefor type safety, consistent with other program declarations.
33-33: Correct initialization pattern.The initialization follows the same pattern as other GPU programs, maintaining consistency in the codebase.
src/logic/types.zig (1)
38-46: Well-designed struct for icon metadata.The
IconDatastruct provides a clear separation between normalized coordinates/dimensions and real dimensions, which is essential for proper texture mapping and rendering. The field types are appropriate and consistent with existing structures.src/run.ts (2)
2-8: LGTM - Clean integration following existing patterns.The
drawMSDFimport follows the established import pattern and maintains consistency with other GPU program functions.
34-35: LGTM - Consistent method signature and implementation.The
draw_msdfmethod follows the same pattern asdraw_texture, using identical parameters and maintaining consistency with the existing GPU program interface.src/index.ts (2)
15-16: LGTM - Proper asset imports.The import statements correctly reference the MSDF output directory, maintaining the build pipeline integration.
60-60: LGTM - Smart texture slot reservation.Reserving the first texture slot for icons prevents index conflicts and ensures consistent texture ID assignment.
src/logic/index.d.ts (2)
32-32: LGTM - Correct method signature for MSDF drawing.The
draw_msdfmethod signature matches the pattern of other GPU program methods and aligns with the implementation insrc/run.ts.
42-42: LGTM - Correct function signature for icon import.The
import_iconsfunction signature correctly declares the expected number array parameter that matches the usage insrc/index.ts.src/types.ts (1)
7-12: Well-designed geometric types for vector rendering.The new type definitions are mathematically correct and provide a clean abstraction for different curve types:
QuadraticBeziercorrectly defines 3 control pointsCubicBeziercorrectly defines 4 control pointsSegmentprovides a unified interface with length metadataThese types will effectively support the MSDF vector rendering pipeline.
src/suspended/rendering vectors/index.ts (1)
7-11: Canvas dimensions will be 0 and element is never removed.The canvas element is created and immediately its dimensions are set to
clientWidth/clientHeight, which will be 0 since the element hasn't been rendered yet. Additionally, the canvas is appended to the document but never removed, causing a memory leak.Fix the dimension issue and ensure cleanup:
const testCanvas = document.createElement('canvas') testCanvas.style.position = 'absolute' + testCanvas.style.width = '100%' + testCanvas.style.height = '100%' document.body.appendChild(testCanvas) - testCanvas.width = testCanvas.clientWidth - testCanvas.height = testCanvas.clientHeight + // Force layout to get actual dimensions + testCanvas.width = testCanvas.offsetWidth || 800 + testCanvas.height = testCanvas.offsetHeight || 600 + + // Clean up canvas after rendering + setTimeout(() => testCanvas.remove(), 0)Likely an incorrect or invalid review comment.
msdf/generate-msdf.js (1)
16-18: Usepath.joinfor better cross-platform compatibility.The current implementation could have issues on different platforms.
function getPath(pathname) { - return path.resolve(__dirname, pathname) + return path.join(__dirname, pathname) }Likely an incorrect or invalid review comment.
package.json (1)
27-27: Format script scope for integration-tests confirmedThe
integration-testsdirectory exists and contains TypeScript files (index.ts,tests/creator.spec.ts), so including it in theformatscript will ensure those files are autoformatted. If that was your intention, no further changes are needed.• package.json (line 27):
"format": "eslint --fix \"src/**/*.{ts,js}\" \"integration-tests/**/*.{ts,js}\""src/WebGPU/programs/draw/getProgram.ts (1)
4-4: Complete the stride calculation.The comment suggests additional vertex attributes are planned but not implemented.
Are the commented values (
+ 1 + 1 + 4) for future vertex attributes? If not needed, remove the comment to avoid confusion.src/suspended/rendering vectors/generateMSDF/getProgram.ts (1)
1-97: Note: This is experimental code in a suspended directory.This file is located in the
suspendeddirectory, indicating it's experimental or not currently active. Ensure this code is properly tested and reviewed before moving it to production.Based on the TypeScript configuration, suspended directories are excluded from the build. Is this intentional, or should this code be integrated into the main codebase?
src/WebGPU/programs/drawMSDF/shader.wgsl (1)
1-39: MSDF shader implementation looks correct.The shader properly implements MSDF rendering with:
- Correct median calculation for multi-channel distance fields
- Proper use of screen pixel distance for edge anti-aliasing
- Standard vertex transformation and texture sampling
src/suspended/rendering vectors/generateMSDF/shader_gemini_better.wgsl (2)
17-22: LGTM!The vertex shader correctly transforms positions and passes the original position with appropriate offset for fragment shader processing.
33-41: LGTM!The median function correctly handles all ordering cases to return the middle value.
src/suspended/rendering vectors/generateMSDF/shader_gemini_freedom_solution.wgsl (5)
17-22: LGTM!Vertex shader implementation is correct.
25-27: Excellent branchless median implementation!This branchless approach using min/max operations is more efficient on GPUs than the branching version in other shader files.
30-45: LGTM!The cubic Bezier evaluation and derivative functions are correctly implemented.
48-82: Well-implemented Newton's method for closest point calculation!This approach is more efficient and accurate than the sampling method used in other shader variants. The handling of edge cases when t is outside [0,1] is correct.
86-121: LGTM!Good use of the
evalBezierfunction to avoid code duplication. The winding number calculation is correct.src/suspended/rendering vectors/generateMSDF/shader.wgsl (4)
60-65: LGTM!Good practice to use epsilon for floating-point comparisons.
68-84: LGTM!Excellent handling of degenerate cases where the tangent vector is zero.
120-193: Well-implemented signed distance calculation!This implementation closely follows the MSDFGen algorithm with proper iterative refinement for accurate distance calculation.
289-298: Uncommented code for perpendicular distance adjustment.This perpendicular distance adjustment is part of the MSDFGen algorithm but is commented out. Is this intentional?
Please verify if the perpendicular distance adjustment should be enabled for accurate MSDF generation.
src/suspended/rendering vectors/generateMSDF/our_shader.wgsl (2)
67-103: LGTM!Clean implementation of pseudo-distance calculation using Newton's method with proper endpoint handling.
340-343: Critical: All channels use the same edge index!All three distance calculations use
minGreenEdgeinstead of their respective edge indices. This will produce incorrect MSDF output.minRedDistance = pseudoDistance(in.originalPosition, minRedEdge); - minGreenDistance = pseudoDistance(in.originalPosition, minGreenEdge); - minBlueDistance = pseudoDistance(in.originalPosition, minBlueEdge); + minGreenDistance = pseudoDistance(in.originalPosition, minGreenEdge); + minBlueDistance = pseudoDistance(in.originalPosition, minBlueEdge);Likely an incorrect or invalid review comment.
| export default function getSvgSource(svgSrc: string) { | ||
| const canvas = document.createElement('canvas') | ||
| const ctx = canvas.getContext('2d')! | ||
| const img = new Image() | ||
| img.onload = function () { | ||
| ctx.drawImage(img, 0, 0) | ||
| } | ||
| img.src = svgSrc | ||
| return canvas | ||
| } |
There was a problem hiding this comment.
Fix asynchronous handling and canvas configuration.
The function has several critical issues:
- Async/sync mismatch: The function returns the canvas immediately, but
img.onloadis asynchronous. The returned canvas will be empty initially. - Missing canvas dimensions: No width/height set (defaults to 300x150).
- No error handling: No handling for image load failures or context creation issues.
- Unsafe context assertion: The
!operator could throw if 2D context is unavailable.
Consider refactoring to use async/await pattern:
-export default function getSvgSource(svgSrc: string) {
- const canvas = document.createElement('canvas')
- const ctx = canvas.getContext('2d')!
- const img = new Image()
- img.onload = function () {
- ctx.drawImage(img, 0, 0)
- }
- img.src = svgSrc
- return canvas
-}
+export default function getSvgSource(svgSrc: string, width = 300, height = 150): Promise<HTMLCanvasElement> {
+ return new Promise((resolve, reject) => {
+ const canvas = document.createElement('canvas')
+ canvas.width = width
+ canvas.height = height
+
+ const ctx = canvas.getContext('2d')
+ if (!ctx) {
+ reject(new Error('Failed to get 2D context'))
+ return
+ }
+
+ const img = new Image()
+ img.onload = () => {
+ ctx.drawImage(img, 0, 0, width, height)
+ resolve(canvas)
+ }
+ img.onerror = () => reject(new Error('Failed to load SVG'))
+ img.src = svgSrc
+ })
+}📝 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.
| export default function getSvgSource(svgSrc: string) { | |
| const canvas = document.createElement('canvas') | |
| const ctx = canvas.getContext('2d')! | |
| const img = new Image() | |
| img.onload = function () { | |
| ctx.drawImage(img, 0, 0) | |
| } | |
| img.src = svgSrc | |
| return canvas | |
| } | |
| export default function getSvgSource( | |
| svgSrc: string, | |
| width = 300, | |
| height = 150 | |
| ): Promise<HTMLCanvasElement> { | |
| return new Promise((resolve, reject) => { | |
| const canvas = document.createElement('canvas') | |
| canvas.width = width | |
| canvas.height = height | |
| const ctx = canvas.getContext('2d') | |
| if (!ctx) { | |
| reject(new Error('Failed to get 2D context')) | |
| return | |
| } | |
| const img = new Image() | |
| img.onload = () => { | |
| ctx.drawImage(img, 0, 0, width, height) | |
| resolve(canvas) | |
| } | |
| img.onerror = () => reject(new Error('Failed to load SVG')) | |
| img.src = svgSrc | |
| }) | |
| } |
🤖 Prompt for AI Agents
In src/utils/getSvgSource.ts lines 1 to 10, the function returns the canvas
immediately without waiting for the asynchronous image load, causing an empty
canvas; also, it lacks canvas width and height settings, error handling for
image load failures and context creation, and uses an unsafe non-null assertion
for the 2D context. Refactor the function to be async, returning a Promise that
resolves after the image loads and is drawn on the canvas; set explicit canvas
dimensions based on the image or SVG source; add error handling for image
loading errors and context retrieval failures; and safely check for the 2D
context without using the non-null assertion operator.
| interface Point { | ||
| x: number | ||
| y: number | ||
| } |
There was a problem hiding this comment.
Remove duplicate Point interface definition.
The Point interface is already defined in src/types.ts (as shown in the relevant code snippets). This duplicate definition could cause TypeScript conflicts.
-interface Point {
- x: number
- y: number
-}
-Import the Point interface from src/types.ts instead:
+import { Point } from '../types'📝 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.
| interface Point { | |
| x: number | |
| y: number | |
| } | |
| import { Point } from '../types' |
🤖 Prompt for AI Agents
In src/logic/index.d.ts at lines 1 to 4, remove the duplicate definition of the
Point interface and instead import the Point interface from src/types.ts to
avoid TypeScript conflicts caused by multiple definitions.
| <path d="M86.467,29.511C82.472,23.734 73.515,15.84 65.626,11.626C53.545,5.174 39.264,3.83 32.01,10.728C19.493,22.63 48.138,36.888 12.048,58.79C-9.698,71.987 26.544,106.787 62.514,97.022C98.483,87.256 99.853,48.867 86.467,29.511Z"/> | ||
| </svg> | ||
| ` | ||
| function testSvgToSegments(): number[] { |
There was a problem hiding this comment.
Add missing imports for svgToSegments and Point.
The function uses svgToSegments and Point type but doesn't import them.
Add the missing imports at the top of the file:
+import svgToSegments from './svgToSegments'
+import type { Point } from '../../types'
+
const TestShapeSvg = `🤖 Prompt for AI Agents
In src/suspended/rendering vectors/index.ts at line 6, the function
testSvgToSegments uses svgToSegments and the Point type without importing them.
Add import statements at the top of the file to import svgToSegments and Point
from their respective modules to ensure the function compiles and runs
correctly.
| .on('finish', function () { | ||
| const ttf = svg2ttf(fs.readFileSync(getPath('output/icons.svg'), 'utf8'), {}) | ||
| fs.writeFileSync(getPath('output/icons.ttf'), ttf.buffer) | ||
|
|
||
| generateBMFont( | ||
| getPath('output/icons.ttf'), | ||
| { | ||
| outputType: 'json', | ||
| charset: [String.fromCharCode(0xe001), String.fromCharCode(0xe002)], | ||
| }, | ||
| (error, textures, font) => { | ||
| if (error) throw error | ||
| textures.forEach((texture, index) => { | ||
| fs.writeFile(texture.filename + '.png', texture.texture, (err) => { | ||
| if (err) throw err | ||
| }) | ||
| }) | ||
| fs.writeFile(font.filename, font.data, (err) => { | ||
| if (err) throw err | ||
| }) | ||
| } | ||
| ) | ||
| }) | ||
| .on('error', function (err) { | ||
| console.log(err) | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for file operations.
The file write operations lack proper error handling which could lead to silent failures.
fontStream
.pipe(fs.createWriteStream(getPath('output/icons.svg')))
.on('finish', function () {
- const ttf = svg2ttf(fs.readFileSync(getPath('output/icons.svg'), 'utf8'), {})
- fs.writeFileSync(getPath('output/icons.ttf'), ttf.buffer)
+ try {
+ const svgContent = fs.readFileSync(getPath('output/icons.svg'), 'utf8')
+ const ttf = svg2ttf(svgContent, {})
+ fs.writeFileSync(getPath('output/icons.ttf'), ttf.buffer)
+ } catch (error) {
+ console.error('Failed to convert SVG to TTF:', error)
+ process.exit(1)
+ }📝 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.
| .on('finish', function () { | |
| const ttf = svg2ttf(fs.readFileSync(getPath('output/icons.svg'), 'utf8'), {}) | |
| fs.writeFileSync(getPath('output/icons.ttf'), ttf.buffer) | |
| generateBMFont( | |
| getPath('output/icons.ttf'), | |
| { | |
| outputType: 'json', | |
| charset: [String.fromCharCode(0xe001), String.fromCharCode(0xe002)], | |
| }, | |
| (error, textures, font) => { | |
| if (error) throw error | |
| textures.forEach((texture, index) => { | |
| fs.writeFile(texture.filename + '.png', texture.texture, (err) => { | |
| if (err) throw err | |
| }) | |
| }) | |
| fs.writeFile(font.filename, font.data, (err) => { | |
| if (err) throw err | |
| }) | |
| } | |
| ) | |
| }) | |
| .on('error', function (err) { | |
| console.log(err) | |
| }) | |
| .on('finish', function () { | |
| try { | |
| const svgContent = fs.readFileSync(getPath('output/icons.svg'), 'utf8') | |
| const ttf = svg2ttf(svgContent, {}) | |
| fs.writeFileSync(getPath('output/icons.ttf'), ttf.buffer) | |
| } catch (error) { | |
| console.error('Failed to convert SVG to TTF:', error) | |
| process.exit(1) | |
| } | |
| generateBMFont( | |
| getPath('output/icons.ttf'), | |
| { | |
| outputType: 'json', | |
| charset: [String.fromCharCode(0xe001), String.fromCharCode(0xe002)], | |
| }, | |
| (error, textures, font) => { | |
| if (error) throw error | |
| textures.forEach((texture, index) => { | |
| fs.writeFile(texture.filename + '.png', texture.texture, (err) => { | |
| if (err) throw err | |
| }) | |
| }) | |
| fs.writeFile(font.filename, font.data, (err) => { | |
| if (err) throw err | |
| }) | |
| } | |
| ) | |
| }) | |
| .on('error', function (err) { | |
| console.log(err) | |
| }) |
🤖 Prompt for AI Agents
In msdf/generate-msdf.js between lines 26 and 51, the file write operations
using fs.writeFile lack proper error handling, which can cause silent failures.
Replace fs.writeFile calls with fs.writeFileSync or use asynchronous
fs.writeFile with proper error callbacks that log or handle errors explicitly.
Ensure all file operations check for errors and handle them appropriately to
avoid silent failures.
| pass.setVertexBuffer(0, vertexBuffer) | ||
|
|
||
| matrixValue.set(worldProjectionMatrix) | ||
| screenPixelDistanceValue.set([16.0]) // Set the screen pixel distance to 1.0 for now, can be adjusted later |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make screen pixel distance configurable.
The screen pixel distance is hardcoded to 16.0, which may not be appropriate for all display densities and zoom levels. This value affects the sharpness of MSDF rendering.
Consider making this value configurable:
- screenPixelDistanceValue.set([16.0]) // Set the screen pixel distance to 1.0 for now, can be adjusted later
+ // Calculate based on device pixel ratio and current zoom level
+ const pixelDistance = 1.0 / (window.devicePixelRatio || 1.0);
+ screenPixelDistanceValue.set([pixelDistance])📝 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.
| screenPixelDistanceValue.set([16.0]) // Set the screen pixel distance to 1.0 for now, can be adjusted later | |
| // Calculate based on device pixel ratio and current zoom level | |
| const pixelDistance = 1.0 / (window.devicePixelRatio || 1.0); | |
| screenPixelDistanceValue.set([pixelDistance]) |
🤖 Prompt for AI Agents
In src/WebGPU/programs/drawMSDF/getProgram.ts at line 107, the
screenPixelDistanceValue is hardcoded to 16.0, limiting flexibility for
different display densities and zoom levels. Refactor the code to accept screen
pixel distance as a configurable parameter or option, allowing it to be set
dynamically based on context or user input instead of a fixed value.
| // Extension from start point (p0) using tangent at t=0 | ||
| let tangentAtStart = 3.0 * (p1 - p0); | ||
| if (length(tangentAtStart) > 0.001) { | ||
| pointOnCurve = p0 + t * normalize(tangentAtStart) * length(tangentAtStart); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove redundant normalize and length operations.
The expression normalize(tangentAtStart) * length(tangentAtStart) is equivalent to tangentAtStart for non-zero vectors.
Simplify the linear extension calculations:
- pointOnCurve = p0 + t * normalize(tangentAtStart) * length(tangentAtStart);
+ pointOnCurve = p0 + t * tangentAtStart;Apply the same simplification to lines 87, 96, and 101.
📝 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.
| pointOnCurve = p0 + t * normalize(tangentAtStart) * length(tangentAtStart); | |
| pointOnCurve = p0 + t * tangentAtStart; |
🤖 Prompt for AI Agents
In src/suspended/rendering vectors/generateMSDF/shader_gemini_better.wgsl at
lines 82, 87, 96, and 101, the expression normalize(tangentAtStart) *
length(tangentAtStart) is redundant and equivalent to tangentAtStart for
non-zero vectors. Simplify these calculations by replacing the expression with
tangentAtStart directly to remove unnecessary normalize and length calls.
|
|
||
| // Sample the curve with extended range for pseudo-distance | ||
| // We extend beyond [0,1] to include the infinite extensions | ||
| let samples = u32(curve_length); |
There was a problem hiding this comment.
Add protection against zero curve length.
If curve_length is 0, this will result in 0 samples and the loop won't execute, potentially leaving minDistance uninitialized for actual use.
Consider adding a minimum sample count:
- let samples = u32(curve_length);
+ let samples = max(u32(curve_length), 16u);📝 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.
| let samples = u32(curve_length); | |
| let samples = max(u32(curve_length), 16u); |
🤖 Prompt for AI Agents
In src/suspended/rendering vectors/generateMSDF/shader_gemini_better.wgsl at
line 55, the assignment of samples from curve_length does not handle the case
when curve_length is zero, which can cause minDistance to remain uninitialized.
Modify the code to ensure samples is at least 1 by using a max operation or
conditional to set samples to 1 if curve_length is zero, preventing zero
iterations in the loop and ensuring minDistance is properly initialized.
| return vec4f(r.minDistance.distance / 10.0, g.minDistance.distance / 10.0, b.minDistance.distance / 10.0, 1.0); | ||
| // return vec4f(r.minDistance.distance / 10.0, g.minDistance.distance / 10.0, b.minDistance.distance / 10.0, 1.0); | ||
| } |
There was a problem hiding this comment.
Remove duplicate line.
Line 303 is a duplicate of line 302.
return vec4f(r.minDistance.distance / 10.0, g.minDistance.distance / 10.0, b.minDistance.distance / 10.0, 1.0);
- // return vec4f(r.minDistance.distance / 10.0, g.minDistance.distance / 10.0, b.minDistance.distance / 10.0, 1.0);🤖 Prompt for AI Agents
In src/suspended/rendering vectors/generateMSDF/shader.wgsl around lines 302 to
304, there is a duplicate return statement on line 303 that repeats the code on
line 302. Remove the duplicate line 303 to avoid redundancy and keep only one
return statement.
| // First pass: Find the closest edge for each channel based on a rough distance. | ||
| // This is an optimization to avoid running signedPseudoDistance for every edge. | ||
| for (var i: u32 = 0u; i < curvesCount; i = i + 1u) { | ||
| // A simple distance check to the curve's start point as a heuristic | ||
| let dist = distance(quadBezierCurves[i][0], in.originalPosition); | ||
|
|
||
| let channel = i % 3u; | ||
| if (channel == 0u) { // Red channel | ||
| if (dist < minRedDist) { | ||
| minRedDist = dist; | ||
| minRedEdge = i; | ||
| } | ||
| } else if (channel == 1u) { // Green channel | ||
| if (dist < minGreenDist) { | ||
| minGreenDist = dist; | ||
| minGreenEdge = i; | ||
| } | ||
| } else { // Blue channel | ||
| if (dist < minBlueDist) { | ||
| minBlueDist = dist; | ||
| minBlueEdge = i; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Optimization may miss the actual closest curve.
Using only the distance to the curve's start point (p0) as a heuristic may not accurately identify the closest curve, especially for curves that bend away from their start points.
Consider using a more robust heuristic, such as:
- Checking distance to both endpoints
- Using curve bounding box distance
- Sampling a few points along the curve
🤖 Prompt for AI Agents
In src/suspended/rendering vectors/generateMSDF/shader_gemini.wgsl around lines
188 to 211, the current heuristic uses only the distance to the curve's start
point to find the closest edge, which can miss the actual closest curve. To fix
this, enhance the heuristic by also checking the distance to the curve's end
point, or by computing the distance to the curve's bounding box, or by sampling
multiple points along the curve and using the minimum distance among these
samples to better approximate the closest edge.
|
🎉 This PR is included in version 0.1.0-next.4 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Refactor