trunk: incorrect gradient transform on copy&paste (rev >= 13047)

[David Mathog]

At revision 13047 text which is filled or stroked with a gradient does not copy properly. What happens is that an extraneous

       gradientTransform="translate((value),(value))"

gets added to the copied gradient.

In the attached example the top gradient text is the original and the two below are copies. Right after they were made both of the copies appeared to be solid, but I then hand edited the SVG on one to remove the gradientTransform line, after which the desired gradient returned. (Demonstrating that the only problem is the one extra line.)

[su_v (suv-lp)]

Reproduced with r13047 on OS X 10.7.5.

[su_v (suv-lp)]

Not limited to gradients on text objects: can be reproduced by pasting a simple rectangle with a gradient.

[insaner (insaner)]

ok, here's what's going on. If you draw a square in the middle of the page, then add a radial gradient (it's more obvious when you use a radial) then copy and paste.. you will notice that the pasted object might appear to be empty. It isn't, just that the gradient's center is offset by a lot (double click on the pasted object to see where it is).

Now, move your original object to have its top left corner aligned with the top left corner of the page, and copy paste.. the new object seems to have no problem!! So, it seems the problem is that somewhere in the copy pasting process, the new object's gradient's center is calculated as being offset from its parent object's center by the same vector that the original object's gradient's center is offset from the top left corner of the page.

In other words, the gradientTransform() that David Mathog mentioned is being added is that of the vector to the original object's gradient's center from the top left corner of the page.

The question, of course, is how/where is this happening, since my code didn't touch any of the vector/offsetting stuff nor does it try to add any gradientTransform()s. I'll see if I can narrow it down, but if anyone has any ideas, please do share.

[su_v (suv-lp)]

Another unexpected side-effect of rev 13047 on copy&pasting of objects with gradients:
Named gradients with identical color stops get merged on paste (i.e. pasted objects are not necessarily sharing the same named gradient definition as the original):

0) launch current trunk with default (new) prefs
1) draw a rectangle
2) draw a second rectangle
3) switch to the gradient tool and create a default gradient for each of them (double-click with the gradient tool)
4) name the gradients differently for each rect (in Fill&Stroke)
5) copy each rect individually
--> the incorrect gradient transformation for both pasted objects is the identical - relative to each pasted object
--> the pasted objects unexpectedly share a single gradient instead of referring to the named gradient used by the original object
    (also reflected in the gradient count)
6) select both original gradients and copy&paste them as selection
--> the gradient handles of the two pasted objects have merged into the same (incorrect) position, and differ relative to the individual pasted object
--> all pasted gradients share the same single gradient now, even if two separately named gradients are in use by the original objects

[su_v (suv-lp)]

Copy & pasting objects with gradients is a very basic feature, and with default settings the pasted object should keep their appearance (gradients keep size and position relative to the pasted object).

[jazzynico (jazzynico)]

Also reproduced on Windows XP, Inkscape trunk revision 13122. Very annoying indeed.

[David Mathog (mathog)]

I don't get this...

updated all the way to r13122 and was seeing the problem. Decided to go looking for the revision that caused it.

bzr revert -r12300
make
src/inkscape #problem not seen
(binary search working upwards)
bzr revert -r13060
make
src/inkscape #problem not seen, should have been (>13047)
make clean
make
src/inkscape #problem not seen
bzr revert -r13047
make clean
make
src/inkscape #problem not seen - on the same release where it first showed up.

Something seems to have changed so that I can no longer reproduce the problem.
Any ideas what that might be? The only thing that I can think of is that recently it was necessary to do:

  ./configure --disable-strict-build

and there were probably object files still around built with make files previous to that. So a mix of object files built with and without that switch. After the make clean, they should all be from "disable-strict-build".

Anybody else see this?

[David Mathog (mathog)]

rebuilt r13126 and the problem is back.

Sometimes.

Inkscape gets into a states where every gradient copied works, then it goes into a state where none of them work. I have not yet been able to resolve what changes, possibly nothing, it may be using an uninitialized variable.

Nothing more fun than trying to track down variable errors. Valgrind time...

[David Mathog (mathog)]

This seems to fix it for me and didn't trigger the crash with the first test file in bug #1171109.

The problem was that the comparison "isEquivalent" for gradients was only testing stop information, it didn't look at the transform at all.

[David Mathog (mathog)]

Added note - I added a separate call to check the transform since there might be some cases where one really does only want to compare the stops.

[su_v (suv-lp)]

Quick test of r13132 + 'changes_2014_03_10a_gradientcopy.patch' on OS X 10.7.5 looks good to me (visual results are ok, copied gradients are merged aka shared as expected). I didn't spend time though to look out for unintended side-effects / regressions.

[jazzynico (jazzynico)]

Also tested successfully on Crunchbang Waldorf, Inkscape trunk revision 13135. No regression found so far.

[David Mathog (mathog)]

Will one of you please commit it? We will find out soon enough if there is some other problem, and not being able to copy and paste gradients is a pretty big bug. Thanks.

[insaner (insaner)]

hi, sorry, I didn't realize I wasn't subscribed to this bug.

 I can confirm that the patch at comment #11 solves the transform issues caused by the fix for 1171109, without causing the issue again. r13047 on fedora 14 linux.

Didn't test too rigorously either though, just fyi.

[insaner (insaner)]

I will go ahead and commit.

[insaner (insaner)]

committed as: r13139

[su_v (suv-lp)]

Here's an example which is not fixed yet (works with r13044, fails with rev >= 13047)

1) download this sample file to your disk:
   <https://launchpadlibrarian.net/35904296/inkscape_bug.svg>
1) launch current trunk with default prefs, new default document
2) import the downloaded file into the new document (File > Import)

Expected result:
The gradients in the imported file have the same orientation as in original.

Actual result:
The orientation of the linear gradients in the imported drawing does not match the original.

Proposing to reopen (based on tests with archived builds works with rev <= 13044, fails with rev >= 13047).

[David Mathog (mathog)]

I see the problem, working on it.

[David Mathog (mathog)]

This should do it.

The issue was that the transform was being checked but not the x1,x2 etc. The equivalent values for radial and mesh gradients were also unchecked. Those are all handled now. Additionally nothing was checking if the types matched (radial vs. linear.)

This patch replaces changes_2014_03_10a_gradientcopy.patch.

[su_v (suv-lp)]

> This patch replaces changes_2014_03_10a_gradientcopy.patch.

Any chance you could provide a patch against current trunk? 'changes_2014_03_10a_gradientcopy.patch' was committed in revision 13139, and 'changes_2014_03_13a_gradientcopy.patch' doesn't apply cleanly to current trunk (r13144)?

[David Mathog (mathog)]

This patch is against r13144

[David Mathog (mathog)]

There was one odd glitch on the way to making changes_2014_03_13c_gradientcopy.patch.

On "bzr update" from 13139 to 13144 the isAligned() function in sp-gradient.cpp was present twice: one copy with the 2014_03_11a changes, and one copy with the 2014_03_10a changes. This wouldn't compile, of course, since the second one redefined an existing function. I have absolutely no idea why that happened.

changes_2014_03_13c_gradientcopy.patch is the diff after hand editing out the second (older) copy of isAligned().

[su_v (suv-lp)]

Committed in r13145 - thx a lot!

[su_v (suv-lp)]

@David - sorry, I saw your last message only after having tested the second patch (compiled ok, results ok) and committing it.

I do get the same results when applying 'changes_2014_03_13a_gradientcopy.patch' to revision 13138 (new local branch), and then pulling up to rev 13144: the function SPGradient::isAligned(SPGradient *that) is defined twice in 'src/sp-gradient.cpp'. Odd indeed.

Could you please verify the correctness of the version of 'src/sp-gradient.cpp' in current trunk (r13145), just to be sure?
<http://bazaar.launchpad.net/~inkscape.dev/inkscape/trunk/view/13145/src/sp-gradient.cpp>

[insaner (insaner)]

@mathog, I just ran into bug #384688 again, and it feels eerily similar to this one, not sure why (maybe it feels like defs not being updated properly, or something like that). Do you think you could take a look at #384688 and see if perhaps you have some insight as to what might be going on, or better yet, how to fix it?

[David Mathog (mathog)]

@insaner - please continue discussion of bug #384688 in that thread. Thanks.

[su_v (suv-lp)]

Follow-up report (regression):
- Bug # “trunk: swatches are duplicated again on paste and import (rev >= 13139)”
  <https://bugs.launchpad.net/inkscape/+bug/1298967>

[su_v (suv-lp)]

Follow-up report (regression on drag&drop or import):
- Bug #1318657 “Drag&Drop imports wrongly gradients (leads to crash)(rev >= 13139)”
  <https://bugs.launchpad.net/inkscape/+bug/1318657>