[go: up one dir, main page]

Import refactor

karliss1requested to merge
karliss1/inkscape:import-refactor into master

Summary

As discussed in mr#7360 there are currently 3 different functions containing copy pasted logic for importing svg document into current document. There should be a single UI independent function implementing the core document import logic that can also be reused for other purpose like mr#7360 .

  • sp_import_document
    • regular paste
    • insertSymbol
  • file_import
    • File/Import
    • drag and drop
    • ClipboardManagerImpl::_pasteImage
  • file_import_pages
    • triggered by file_import on certain file types when choosing "Add SVG as new page(s) in the current file"

Explanation for some of changes

Removed the special handling for <use> tags. It was supposed to fix clone positioning after copy but that code hasn't done anything for a long time. When making a copying inkscape adds additional groups, so the clones would never be at root. Even if inkscape didn't add additional groups, clones could be arbitrary deep so the old code wouldn't handle those. The case described in MR which added works correctly even without it. A more complicated case described in "Additional discovered bugs" section behaves equally wrong back to at least 1.1 which if I am not mistaken is first version where the extra case was added.

SVG import related issues:

Not saying this MR will fix them, but they should be at least be considered when rewriting relevant parts of code and retested after MR is done. Also any bugfixes for them will likely conflict with this MR and should better be done once there is single place to implement the fix instead of 3 subtly different each with their own set of special case handling.

Misc:

References:

Transform:

Style attributes:

CSS:

Main categories of bugs:

  • missing/buggy special handling of certain object types
  • pasting objects referencing other objects
  • correct transformation handling (scale, positioning)
  • attributes inherited from parent
    • lost during copy
    • included in clipboard but lost during paste/import
    • unexpected new attributes from the new parent

Not related:

  • copy paste clipboard mime type handling
  • copy/pasting attributes and tool specific non objects within inkscape (nodes, text, style ...)
  • file format reading bugs
  • import used to described File/Open non svg files (instead of File/Import)

Testing scenarios

  • File import
  • File import as pages
  • Paste regular
  • Paste in place
  • Paste on page
  • Paste in same document
  • Paste across documents
  • Drag and drop svg
  • Drag and drop raster image -> should produce single image object without additional groups
  • Paste raster image regular
  • Paste image with specific mime type (the one that triggers ClipboardManagerImpl::_pasteImage)
  • y axis up/down
  • paste/import between documents with different user units
  • documents with offset viewport (broken in 1.4.2)
  • view flip, rotation
  • paste under a parent with non empty transformation
  • copy from a parent with a non empty transformation
  • Look into what's going on with clone pasting/positioning
  • Mouse position grid snapping

Cases fixed by this MR

Additional discovered bugs (also affects master)

Some of them could be fixed relatively easy, but I would like to limit the scope of this MR. And since they also affect master, it's not a regression.

  • Copying clone into document which already contains a copy of original can result in wrong position
  • "Paste in place" and "Paste on page" across documents with different Y axis direction results in wrong position (regular paste always works)
  • Inserting symbol from the lib vs document results in different scale
Edited by karliss1

Merge request reports