Supporting SVG renderer changes for subpixel bounds #127
Conversation
|
This contains potentially breaking changes-- revisit scratchfoundation/scratch-vm#1650 or check that the API surface used by other repos is unaffected |
|
|
||
| const svgData = "<svg>...</svg>"; | ||
| const scale = 1; | ||
| const quirksMode = false; |
There was a problem hiding this comment.
Maybe we should try to document what this means
| @@ -58,8 +58,9 @@ | |||
| } | |||
|
|
|||
| function renderSVGString(str) { | |||
| // Reset the canvas transform after drawing. | ||
| this._context.setTransform(1, 0, 0, 1, 0, 0); | ||
| // Set the CSS style of the canvas to the actual measurements. | ||
| this._canvas.style.width = bbox.width; |
There was a problem hiding this comment.
Note: this was added in scratchfoundation/scratch-render@b35f684
and we don't know why, and it doesn't seem relevant anymore
| */ | ||
| this.adjustedSize = null; | ||
|
|
||
| this.adjustedMeasurements = {xOffset: 0, yOffset: 0, width: 0, height: 0}; |
| * Null when an SVG is not loaded. | ||
| * @type {?Array<number>} | ||
| */ | ||
| this.adjustedRotationCenter = null; |
There was a problem hiding this comment.
Let's think about how we can move the very scratch-render-api specific functions back into Render to make sure that other dependencies aren't accidentally affected
|
|
||
| const measurements = this.measurements; | ||
|
|
||
| // Compensate for quirks-mode SVG viewbox offset. |
There was a problem hiding this comment.
Remove reference to "quirks-mode" here, since that's sprite2 specific
| /** | ||
| * Set the loaded SVG's viewBox and this renderer's viewOffset for proper subpixel positioning. | ||
| * @param {Array<number>} rotationCenter The rotation center of the Skin this SVG is used for. | ||
| * @param {number} minScale The minimum scale at which this SVG should be rendered with proper subpixel positioning. |
There was a problem hiding this comment.
Sounds like this is specifically the minimum mipmap scale
There was a problem hiding this comment.
Also, the scale needs to be 1/2^x form
|
|
||
| this.adjustedSize = adjustedSize; | ||
|
|
||
| this.adjustedMeasurements.xOffset = ((1 - centerFrac[0]) / minScale); |
| // and with integer dimensions at power-of-two sizes down to minScale. | ||
| this._svgTag.setAttribute('viewBox', | ||
| `${ | ||
| measurements.x - ((1 - centerFrac[0]) / minScale) |
There was a problem hiding this comment.
Change to use adjustedMeasurements
|
This logic can now all be moved back into scratch-render |
Resolves
Half of resolving scratchfoundation/scratch-render#427
Proposed Changes
_drawandfromStringmethodsmeasurementspublic and addadjustedRotationCenter,adjustedSize, andadjustedMeasurementsloadSVG, makecreateSVGImagepublic, and addsetSubpixelViewboxviewOffsetReason for Changes
These changes implement the SVG side of the changes described in scratchfoundation/scratch-render#550. They allow for the bounding box and rotation center of a loaded SVG to be adjusted so that it can be rendered at an integer size and integer coordinates.
The supporting API changes give scratch-render more control over what happens when, and were done as necessary to make things function in scratch-render. These more tightly couple scratch-render and this repo, but I'm not sure how else to implement this stuff--any suggestions on reducing coupling are greatly appreciated.
Test Coverage
Still needs some discussion
To do
adjustedSizeand just grab width/height fromadjustedMeasurementsSvgRenderer.sizeor if the renderer can grab that frommeasurements