Pen #72
Merged
Pen #72
Conversation
This file contains 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
added 12 commits
December 15, 2016 13:35
This is the absolute basics of a working pen layer. Still to do: - Make it so that the pen layer can be in the drawables list so that it can be in front of the backdrop but behind other Drawables. - Remove current debug canvas operations - Add pen layer API
The skin-based implementation of `Drawable` has been moved into a new class called `SkinnedDrawable`. This makes room for `PenLayer` to become a new type of `Drawable`.
It turns out that it might make sense for the pen to share these features. Splitting them out would likely work too, but would introduce unnecessary complexity.
To test, load the playground and type `renderer.createPenDrawable()` into the JS console. Note that the pen layer is now just another drawable object, so it can move and use effects.
The `setStageSize` wasn't working correctly. This change fixes that and also notifies active `PenDrawable` instances about the resize. This was implemented by making the renderer an `EventEmitter`.
Merge the "skin classes" work from another branch and move/convert the pen work from `PenDrawable` to `PenSkin`.
Also clean up the code around our calls to `readPixels`: - Use `Uint8Array` instead of `Buffer` to hold the pixels - Use `ImageData.data.set` instead of a `for` loop when copying pixels to a canvas
Scratch uses y=up, whereas WebGL and the Canvas element both use y=down. To make this work, we set a non-conventional projection matrix when rendering to the stage. Some of the internal renders (touching color, stamp, etc.) were using a y-flipped projection matrix, though, which means that the output was double-flipped and would appear incorrect. For the touching-color block this just meant a cosmetic issue: the debug canvas looks upside-down. For pen stamp, though, it's a real issue since the stamp was appearing upside down on the stage. This change ensures that the projection matrix for every internal render follows WebGL / Canvas conventions, and that we only y-flip on the final render to the stage.
Changes include: - Implement pen clear - Upack pen-related location parameters - Fix inverted vertical position on pen actions - Remove canvas debug content - Set `strokeStyle` instead of `fillStyle` - Remove location parameter from stamp operation: always stamp in place
thisandagain
approved these changes
Jan 20, 2017
| this._canvasDirty = true; | ||
| } | ||
|
|
||
| _setAttributes (context, penAttributes) { |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
|
I asked back in the scratch 2 beta for a way to have the pen as the topmost
layer... Oh how useful that would have been! :)
…On 20 Jan 2017 8:56 p.m., "Andrew Sliwinski" ***@***.***> wrote:
***@***.**** approved this pull request.
Really minor notes. This looks great @cwillisf
<https://github.com/cwillisf>
------------------------------
In src/PenSkin.js
<#72 (review)>
:
> + this._rotationCenter[0] = width / 2;
+ this._rotationCenter[1] = height / 2;
+ this._texture = twgl.createTexture(
+ gl,
+ {
+ auto: true,
+ mag: gl.NEAREST,
+ min: gl.NEAREST,
+ wrap: gl.CLAMP_TO_EDGE,
+ src: this._canvas
+ }
+ );
+ this._canvasDirty = true;
+ }
+
+ _setAttributes (context, penAttributes) {
Missing jsdoc.
------------------------------
In src/RenderWebGL.js
<#72 (review)>
:
> @@ -496,17 +540,15 @@ class RenderWebGL {
gl.disable(gl.STENCIL_TEST);
}
- const pixels = new Buffer(bounds.width * bounds.height * 4);
+ const pixels = new Uint8Array(bounds.width * bounds.height * 4);
Do you know what babel will convert this to? Uint8Array is currently only
supported by Chrome and Firefox. Perhaps something we can polyfill?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#72 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AGbNvtPeGB72KFHjwpoJrCVlq__cI0KRks5rUR93gaJpZM4LohZH>
.
|
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.
Resolves
This set of changes adds rendering support for Scratch 2.0-style pen features as a step toward #6.
Proposed Changes
This introduces a new class, called
PenSkin, which acts like an initially invisible "costume" that has pen-related functionality. The renderer's API can be used to create aPenSkinand assign it to aDrawablecorresponding to the pen "layer" in Scratch, then call pen-related methods on the renderer to draw on thePenSkin.Since the
PenSkinis hosted by a regularDrawable, in theory we could implement some features that Scratch 2.0 did not have. This includes multiple pen layers, graphical effects on pen layers, moving pen layers, and more. This might not be desirable in core Scratch 3.0, but I think we should consider allowing extensions access to these features.Reason for Changes
Implementing pen features brings us closer to Scratch 2.0 compatibility.
Test Coverage
The renderer does not currently have tests.