paperjs / paper.js

The Swiss Army Knife of Vector Graphics Scripting – Scriptographer ported to JavaScript and the browser, using HTML5 Canvas. Created by @lehni & @puckey
http://paperjs.org
Other
14.42k stars 1.22k forks source link

globalMatrix of SymbolDefinition is mutated after creating SymbolItems #1684

Open nikkwong opened 5 years ago

nikkwong commented 5 years ago

I have a project where it is important that the matrix of the SymbolDefinition reports proper coordinates. Here is a Sketch showing a SymbolDefinition that is selected = true, but drawing a selection box around the last drawn SymbolItem.

It seems to me it is because _draw() on SymbolItem is mutating the matrix of the SymbolDefinition directly to draw the SymbolItem. A workaround that works in my case is to just reset the matrix of the SymbolDefinition when complete.

_draw: function(ctx, param) {
    this._definition._item.draw(ctx, param);
    // my hotfix to reset matrix to previous state
    this._definition._item._globalMatrix = param.matrices[param.matrices.length - 2]
}
nikkwong commented 5 years ago

Looks like it needs to be done recursively to account for children:

_draw: function(ctx, param) {
    const recursivelyReset = (item, matrix) => {
        item._globalMatrix = matrix
        for (let child of (item.children || []))
            recursivelyReset(child, matrix)
    }
    this._definition._item.draw(ctx, param);
    recursivelyReset(this._definition._item, param.matrices[param.matrices.length - 2])

}

Not sure what other gotchas there are here.

sasensi commented 5 years ago

Hi, about the selection problem, it is a known bug that will be fixed by https://github.com/paperjs/paper.js/pull/1577.

But more generally and linked to my answer about https://github.com/paperjs/paper.js/issues/1683, I think that you have a misconception about how SymbolDefinition is supposed to be used. Once a SymbolDefinition is created from an Item, this Item is purposely removed from the scene graph and it should not be manually reintroduced in it (like you do in your sketch: paper.project.layers[0].addChild(symbol.definition)) or it will lead to unexpected behavior. You should treat SymbolDefinition only as a reference for its linked SymbolItem instances, but not try to manipulate it like any other item. The only thing that you should do with it is manipulating its properties (style, ...) which will be reflected on all linked SymbolItem instances.

So I would say that you shouldn't rely on the SymbolDefinition matrix or coordinates. @lehni, am I right ?

nikkwong commented 5 years ago

I know that I am not using SymbolDefinition the way that @lehni intended. However, in the case that paperJS is being used to create drawing applications, it is not realistic to assume that users will create SymbolDefinition and then never need to change that definition in the future.

The question then is what is the easiest way to edit these definitions. Well, IMO it is easiest to just keep it in the scene graph. Taking the definition out of the scene graph implicitly assumes that any changes to the definition will be made programmatically (without some end user GUI). But if paperJS is being used to build a drawing GUI at that point it makes sense for the definition to also be modifiable in that GUI (and therefore to be made available in the scene graph). For example if the user needs to have access to the symbol to have drag-to-edit segment path data.

To achieve this I just commented out the item.remove() call in the SymbolDefinition constructor. I wasn't thinking about this before but it would be nice to have this available via the API. In either case, making sure the globalMatrix coordinates are being reported correctly is better behavior than being incorrect; especially if it's as easy of a code fix as I've come up with.