✨ feat: high-quality pen#178
Open
SimonShiki wants to merge 8 commits into
Open
Conversation
Signed-off-by: SimonShiki <sinangentoo@gmail.com>
Signed-off-by: SimonShiki <sinangentoo@gmail.com>
Signed-off-by: SimonShiki <sinangentoo@gmail.com>
Signed-off-by: SimonShiki <sinangentoo@gmail.com>
Signed-off-by: SimonShiki <sinangentoo@gmail.com>
Deploying clipcc-preview with
|
| Latest commit: |
76abe4d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://eec48730.clipcc-preview.pages.dev |
| Branch Preview URL: | https://feat-hqpen-2.clipcc-preview.pages.dev |
Signed-off-by: SimonShiki <sinangentoo@gmail.com>
Signed-off-by: SimonShiki <sinangentoo@gmail.com>
Signed-off-by: SimonShiki <sinangentoo@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces an optional “High-Quality Pen” mode which makes pen rendering (lines and stamping) operate in the actual WebGL canvas pixel size (instead of a fixed 480×360 pen buffer), and wires the setting through the GUI into the VM/renderer.
Changes:
- Add a renderer flag + events to toggle HQ pen and react to physical canvas resizing.
- Update
PenSkinto track a separate_canvasSize, scale pen thickness/coordinates, and rebuild buffers when canvas/HQ mode changes. - Add a GUI setting (stored in settings reducer) and propagate it into the VM renderer via
vm-manager-hoc.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/render/src/RenderWebGL.js | Adds HQ-pen toggle, emits canvas-size change event, and adjusts pen stamping viewport/draw options for HQ mode. |
| packages/render/src/RenderConstants.js | Adds new renderer events for canvas size and HQ-pen toggle. |
| packages/render/src/PenSkin.js | Implements HQ pen buffer sizing, scaled line drawing, buffer reset/copy on size changes, and silhouette updates. |
| packages/gui/test/unit/util/vm-manager-hoc.test.jsx | Updates VM renderer mock to include new renderer methods. |
| packages/gui/src/reducers/settings.ts | Adds persisted highQualityPen setting with default false. |
| packages/gui/src/lib/vm-manager-hoc.jsx | Applies/syncs highQualityPen setting to the VM renderer. |
| packages/gui/src/containers/settings-modal.jsx | Plumbs the HQ pen setting through the settings modal container. |
| packages/gui/src/components/settings-modal/settings-modal.jsx | Adds the “High-Quality Pen” toggle UI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+272
to
+285
| const scaleX = this._canvasSize[0] / this._size[0]; | ||
| const scaleY = this._canvasSize[1] / this._size[1]; | ||
|
|
||
| const diameter = penAttributes.diameter || DefaultPenAttributes.diameter; | ||
| const thickness = diameter * Math.min(scaleX, scaleY); | ||
|
|
||
| // Adjust for odd-width lines to align with physical pixel centers | ||
| const offset = (Math.round(thickness) % 2 === 0) ? 0 : 0.5; | ||
|
|
||
| // Scale the points and thickness to the physical canvas size | ||
| const scaledX0 = (x0 * scaleX) + offset; | ||
| const scaledY0 = (y0 * scaleY) + offset; | ||
| const scaledX1 = (x1 * scaleX) + offset; | ||
| const scaledY1 = (y1 * scaleY) + offset; |
Comment on lines
371
to
+374
| const gl = this._renderer.gl; | ||
|
|
||
| const oldTexture = this._texture; | ||
|
|
Comment on lines
+392
to
395
| this._framebuffer = twgl.createFramebufferInfo(gl, attachments, width, height); | ||
|
|
||
| gl.clearColor(0, 0, 0, 0); | ||
| gl.clear(gl.COLOR_BUFFER_BIT); |
Comment on lines
+397
to
420
| if (oldTexture) { | ||
| // Draw old canvas to new framebuffer | ||
| this._renderer.enterDrawRegion(this._usePenBufferDrawRegionId); | ||
| gl.viewport(0, 0, width, height); | ||
|
|
||
| const currentShader = this._renderer._shaderManager.getShader(ShaderManager.DRAW_MODE.default); | ||
| gl.useProgram(currentShader.program); | ||
| twgl.setBuffersAndAttributes(gl, currentShader, this._renderer._bufferInfo); | ||
|
|
||
| const uniforms = { | ||
| u_skin: oldTexture, | ||
| u_projectionMatrix: twgl.m4.ortho(width / 2, width / -2, height / -2, height / 2, -1, 1), | ||
| u_modelMatrix: twgl.m4.scaling(twgl.v3.create(width, height, 0), twgl.m4.identity()) | ||
| }; | ||
|
|
||
| twgl.setTextureParameters(gl, oldTexture, { | ||
| minMag: gl.NEAREST | ||
| }); | ||
| twgl.setUniforms(currentShader, uniforms); | ||
| twgl.drawBufferInfo(gl, this._renderer._bufferInfo, gl.TRIANGLES); | ||
|
|
||
| gl.deleteTexture(oldTexture); | ||
| } | ||
|
|
Comment on lines
1773
to
+1777
| gl.viewport( | ||
| (this._nativeSize[0] * 0.5) + bounds.left, | ||
| (this._nativeSize[1] * 0.5) - bounds.top, | ||
| bounds.width, | ||
| bounds.height | ||
| ((this._nativeSize[0] * 0.5) + bounds.left) * scaleX, | ||
| ((this._nativeSize[1] * 0.5) - bounds.top) * scaleY, | ||
| bounds.width * scaleX, | ||
| bounds.height * scaleY |
Comment on lines
+26
to
28
| * NativeSizeChanged event, which related to stage size change. | ||
| * @constant {string} | ||
| */ |
| autoSave: boolean; | ||
| infiniteCloning: boolean; | ||
| edgelessStage: boolean; | ||
| highQualityPen: boolean, |
Comment on lines
119
to
125
| dispose () { | ||
| this._renderer.removeListener(RenderConstants.Events.NativeSizeChanged, this.onNativeSizeChanged); | ||
| this._renderer.removeListener(RenderConstants.Events.CanvasSizeChanged, this.onCanvasSizeChanged); | ||
| this._renderer.removeListener(RenderConstants.Events.UseHighQualityPenChanged, this.onUseHighQualityPenChanged); | ||
| this._renderer.gl.deleteTexture(this._texture); | ||
| this._texture = null; | ||
| super.dispose(); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Make pen always draw in native canvas size, rather than in hardcoded 480x360 pen canvas.
Proposed Changes
Test Coverage
Not sure what can we do now, but current tests remain passed.
Additional Context
Not sure if it hurts performance too much, but in some major projects that recommended to use HQ pen works great.