Fixes & optimizations #66
Open
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
- Remove inefficient format() for template strings - Remove inefficient lineTo indexOf on large strings
Example: ``` ctx.rect(75, 75, 100, 100) ctx.lineWidth = 30 ctx.strokeStyle = '#0074D9' ctx.stroke() ctx.fillStyle = '#FF4136' ctx.fill() ctx.lineWidth = 10 ctx.strokeStyle = '#FFDC00' ctx.stroke() ```
The while loop was crashing my browser, so converted into a for loop. The issue is when appendChild does not remove the child from __defs.childNodes, I’m not entirely sure why this was happening, but this method also is slightly more optimized as you don’t recalculate the length upon each interation.
When patternUnits uses the default `objectBoundingBox` patterns are parsed incorrectly for me (they do not repeat, and are incorrectly scaled). Adding this line fixes this issue.
- Support multiple fills w/ single path. Ex. ```js ctx.rect(0, 0, 100, 100); ctx.fill(); ctx.translate(50, 50); ctx.fill(); ``` - Support `gradientTransform` and `patternTransform`. Ex. ```js ctx.rect(0, 0, 100, 100); ctx.translate(50, 50); ctx.fillStyle = ctx.createPattern($image, ‘repeat’) ctx.fill(); ``` - Support switching between transforms and path operations. Ex. ```js ctx.rect(0, 0, 100, 100); ctx.translate(50, 50); ctx.rect(0, 0, 100, 100); ctx.fill(); ``` - Fix clearRect() so <svg> is emptied if requested cleared area covers the entire canvas area. This update takes into account the current matrix, as well as accounts for rectangles outside of canvas bounds.
``` ctx.translate(10, 10) ctx.fillRect(0, 0, 10, 10) ctx.save() ctx.restore() ctx.translate(10, 10) ctx.fillRect(0, 0, 10, 10) ```
This patch creates smaller exported svgs by creating <def> entries for paths references more than one. This kicks into gear when a fill or stroke is used multiple times on a single path.
|
I tried this patch because the fill=evenodd bug hit me hard. I can confirm that the evenodd fix works. Unfortunately I also have to report that some of the other optimisations broke other stuff, I get funny results that were previously ok. |
|
Hey jods, what new issues are you noticing with this pull request? Perhaps
I can sort them out if you give me some detailed instructions to reproduce.
…On Sun, Jun 3, 2018, 11:44 AM jods ***@***.***> wrote:
I tried this patch because the fill=evenodd bug hit me hard.
I can confirm that the evenodd fix works.
Unfortunately I also have to report that some of the other optimisations
broke other stuff, I get funny results that were previously ok.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#66 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAGMvOnmhGKR0frkC3vQfK0acdwfu-Rxks5t5C6KgaJpZM4SjoX_>
.
|
|
@mudcube in my document many lines end up at the origin after your patch, they were drawn correctly before. Unfortunately it's real documents with around 10K calls to context, that I cannot easily share or reduce. That said, some tests are currently falling, could that be related? |
- Clip would sometimes report that group was not the parent of currentElement. So, instead calling `.remove()` on the element. - Create path if does not exist when calling `getOrCreateElementToApplyStyleTo` - Revert support switching between transforms and path operations… this needs to be rethought as it was causing other transformation issues.
|
@jods4 The transformation issue should be sorted out, let me know if you're still running into the issue. There's still a couple failing tests, but part of that is due to slight formatting changes (ex. spacing), and doesn't actually affect what's visually exported. |
This allows the intention behind commit c228ba4 to work once again. Note, this commit contains MPL code, so if this pull request is accepted an MPL license would need to be added to the repository.
|
@mudcube I tried again and the "all lines go to origin" problem is fixed on your master. But it's not completely fixed. |
23 tasks
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.
This pull request:
fill-rulesupport as reported in issue ctx.fill("evenodd") doesn't work + fix #62opacitysupport for issue Set image 'opacity' attribute in drawImage method #53