adobe-research / svgObjectModelGenerator

SVG OM Generator & Writer
Apache License 2.0
49 stars 20 forks source link

Crema asserts on text-on-path-2 #146

Closed dirkschulze closed 9 years ago

dirkschulze commented 9 years ago

According to @mvujovic text-on-path-2 fails on Crema with the following error (output from generator):

[info:crema 12:08:32.596 rendermanager.js:157:30] RenderManager active
[info:crema 12:08:32.612 rendermanager.js:179:26] Rendering: Text on Path.svg (2;1)
[info:crema 12:08:32.896 rendermanager.js:179:26] Rendering: Plain text.svg (1;2)
[info:crema 12:08:32.911 rendermanager.js:179:26] Rendering: more path text.svg (0;3)
Ex: TypeError: Cannot read property 'right' of undefined
TypeError: Cannot read property 'right' of undefined
    at SVGWriterPreprocessor.shiftBounds (/Users/mvujovic/Code/svgObjectModelGenerator/svgWriterPreprocessor.js:258:52)
    at SVGWriterPreprocessor.processSVGNode (/Users/mvujovic/Code/svgObjectModelGenerator/svgWriterPreprocessor.js:367:22)
    at SVGWriterPreprocessor.processSVGNode (/Users/mvujovic/Code/svgObjectModelGenerator/svgWriterPreprocessor.js:381:26)
    at Array.forEach (native)
    at SVGWriterPreprocessor.processSVGNode (/Users/mvujovic/Code/svgObjectModelGenerator/svgWriterPreprocessor.js:379:26)
    at SVGWriterPreprocessor.processSVGNode (/Users/mvujovic/Code/svgObjectModelGenerator/svgWriterPreprocessor.js:381:26)
    at Array.forEach (native)
    at SVGWriterPreprocessor.processSVGNode (/Users/mvujovic/Code/svgObjectModelGenerator/svgWriterPreprocessor.js:379:26)
    at SVGWriterPreprocessor.processSVGNode (/Users/mvujovic/Code/svgObjectModelGenerator/svgWriterPreprocessor.js:381:26)
    at Array.forEach (native)
GET /extractasset/258/3/0/Text%20on%20Path.svg?v=1 200 2ms
GET /extractasset/258/4/1/Plain%20text.svg?v=1 200 1ms
Ex: TypeError: Cannot read property 'right' of undefined
TypeError: Cannot read property 'right' of undefined
    at SVGWriterPreprocessor.shiftBounds (/Users/mvujovic/Code/svgObjectModelGenerator/svgWriterPreprocessor.js:258:52)
    at SVGWriterPreprocessor.processSVGNode (/Users/mvujovic/Code/svgObjectModelGenerator/svgWriterPreprocessor.js:367:22)
    at SVGWriterPreprocessor.processSVGNode (/Users/mvujovic/Code/svgObjectModelGenerator/svgWriterPreprocessor.js:381:26)
    at Array.forEach (native)
    at SVGWriterPreprocessor.processSVGNode (/Users/mvujovic/Code/svgObjectModelGenerator/svgWriterPreprocessor.js:379:26)
    at SVGWriterPreprocessor.processSVGNode (/Users/mvujovic/Code/svgObjectModelGenerator/svgWriterPreprocessor.js:381:26)
    at Array.forEach (native)
    at SVGWriterPreprocessor.processSVGNode (/Users/mvujovic/Code/svgObjectModelGenerator/svgWriterPreprocessor.js:379:26)
    at SVGWriterPreprocessor.processSVGNode (/Users/mvujovic/Code/svgObjectModelGenerator/svgWriterPreprocessor.js:381:26)
    at Array.forEach (native)
[info:crema 12:08:34.393 rendermanager.js:191:38] RenderManager idle

Note that the test passes when exporting the whole document. The issue seems to be:

 pR = omIn._parentBounds.right

in svgWriterProcessor.js

imaderyc commented 9 years ago

these 2 layers were created by:

  1. click on Eclipse tool; change to Path at the top; draw circle
  2. click on Text tool; select Right align for text (it does NOT reproduce if the text is left or center aligned)
  3. move mouse to the path on canvas until it shows 'text on path' pointer, type text.
dirkschulze commented 9 years ago

Ok, omIn._parentBounds is just set if omIn.type === "text". We have "textPath" though. Apparently in this.processSVGNode we don't assume that we would need _parentBounds but when we compute the position of text caused by alignment, we require _parentBounds. Setting _parentBounds for "textPath" might not be the solution though.

I don't know why we just hit this when exporting SVG within certain bounds yet.

nimbupani commented 9 years ago

@dirkschulze is this issue fixed? Unclear because the commit says its fixed but this issue is still open.

dirkschulze commented 9 years ago

Lets say it that way, it was broken before in two ways. The crash and the fact that it was not working correctly. The patch fixes the patch. I don't think that it fixes the fact that it was not working properly in the first place.