Hey Bulia,
just found a bug with gradient editing. Steps to reproduce:
1. Create a Text 2. Apply a gradient to the text 3. Delete the text 4. Undo deleting the text 5. Try to edit the gradient or switch to a solid fill. It won't work
If you can't fix it right away, I'll file a bug report against it.
Take care!
David
On Tuesday 8 March 2005 22:14, David Christian Berg wrote:
just found a bug with gradient editing. Steps to reproduce:
- Create a Text
- Apply a gradient to the text
- Delete the text
- Undo deleting the text
- Try to edit the gradient or switch to a solid fill. It won't work
While I was trying this I stumbled across the same behaviour with shapes:
1. Draw a shape 2. Apply a gradient to it, change it a bit for good measure 3. Undo until you're back at stage 1 4. Redo 5. The gradient is now dead in the middle of the shape and when you try to modify it it becomes white.
Jef
On Tue, 8 Mar 2005 23:42:12 +0100, Jean-François Lemaire <jflemaire@...621...> wrote:
- Draw a shape
- Apply a gradient to it, change it a bit for good measure
- Undo until you're back at stage 1
- Redo
- The gradient is now dead in the middle of the shape and when you try
to modify it it becomes white.
And this is actually a different bug. It's not related to my gradient code. By trying timed CVS updates I tracked it down to mental's commit on Tue Mar 1 23:43:25 2005 UTC which touched these files:
P src/xml/Makefile_insert cvs update: src/xml/node-listener.h is no longer in the repository P src/xml/node-observer.h P src/xml/repr.cpp P src/xml/simple-node.cpp P src/xml/simple-node.h
Before that commit redoing undone gradient works, after that it's broken. Mental, could you please look into it?
On Tue, Mar 08, 2005 at 11:13:20PM -0400, bulia byak wrote:
On Tue, 8 Mar 2005 23:42:12 +0100, Jean-François Lemaire <jflemaire@...621...> wrote:
- Draw a shape
- Apply a gradient to it, change it a bit for good measure
- Undo until you're back at stage 1
- Redo
- The gradient is now dead in the middle of the shape and when you try
to modify it it becomes white.
[...] By trying timed CVS updates I tracked it down to mental's commit on Tue Mar 1 23:43:25 2005 UTC which touched these files:
P src/xml/Makefile_insert cvs update: src/xml/node-listener.h is no longer in the repository P src/xml/node-observer.h P src/xml/repr.cpp P src/xml/simple-node.cpp P src/xml/simple-node.h
This was caused by a bug in ListContainer::pop_front, which wasn't correctly adjusting _tail.
I've fixed this, as well as a couple of other bugs I noticed. I also changed check_values to try to test back() as an indirect way of checking that _tail is correct: though note that this won't check _tail correctness if the list is empty. mental, you might consider somehow adding a test that _tail is correct whenever the list is empty. It seems to me that it's fairly easy to forget to adjust _tail correctly.
pjrm.
On Wed, 09 Mar 2005 22:23:51 +1100, Peter Moulder <Peter.Moulder@...38...> wrote:
This was caused by a bug in ListContainer::pop_front, which wasn't correctly adjusting _tail.
Thanks Peter! Actually this has fixed BOTH reported bugs, the first one with undoing text deletion and the second one with redoing gradient creation on shape. Cool :)
On Tue, 08 Mar 2005 22:14:56 +0100, David Christian Berg <david@...407...> wrote:
just found a bug with gradient editing. Steps to reproduce:
- Create a Text
- Apply a gradient to the text
- Delete the text
- Undo deleting the text
- Try to edit the gradient or switch to a solid fill. It won't work
This is another manifestation of this bug:
http://sourceforge.net/tracker/index.php?func=detail&aid=1058285&gro...
Namely when you delete and then undo deletion of text, the deleted copy (or its NRArenaItem) still lingers somewhere and holds a href to the gradient. So the href count after that is always 1 more than the actual usage count of the gradient, and thus the gradient is regenerated anew on each drag.
Mental, do you have any fresh ideas on how to fix it?
Quoting bulia byak <buliabyak@...400...>:
Mental, do you have any fresh ideas on how to fix it?
Yes, finish reworking the NRArenaItems to no longer reference SPStyle (which, in turn, references the gradients...).
This basically means changing the NRArenaItem interface to present a lot of getter/setter methods rather than giving it an SPStyle * and saying "use this".
I've already done a lot of the heavy lifting a long time ago, but it wore me out. Anyone interested in finishing the job?
-mental
On Tue, 8 Mar 2005 18:31:01 -0500, mental@...3... <mental@...3...> wrote:
Yes, finish reworking the NRArenaItems to no longer reference SPStyle (which, in turn, references the gradients...).
Actually, now that I think of it, why do NRArenaItems have to reference style at all? If an NRArenaItem uses it, this means its parent SPItem also uses and references it, so there's no danger it will be deleted from under the NRArenaItem. Can we just remove the referencing? Or am I missing something?
Quoting bulia byak <buliabyak@...400...>:
On Tue, 8 Mar 2005 18:31:01 -0500, mental@...3... <mental@...3...> wrote:
Yes, finish reworking the NRArenaItems to no longer reference SPStyle (which, in turn, references the gradients...).
Actually, now that I think of it, why do NRArenaItems have to reference style at all? If an NRArenaItem uses it, this means its parent SPItem also uses and references it, so there's no danger it will be deleted from under the NRArenaItem. Can we just remove the referencing? Or am I missing something?
For various architectural reasons, there's a delay between the parent SPItem going away and the NRArenaItem going away. If the SPStyle weren't reffed by the NRArenaItem, it would get freed early most likely resulting in a (hard-to-reproduce) crash.
[ As a rule, "removing the referencing" when there is still an outstanding pointer is never a fix... ]
Now, what might be possible is if the SPItem set the NRArenaItem's style to NULL or something (if NRArenaItem were rewritten to deal with a NULL style).
But I'd rather just finish the job of detangling NRArenaItem and SPStyle anwyay... it's one of the biggest roadblocks to cleaning up the SPStyle code.
Mostly it just means the NRArenaItem (or NRArenaItem subclass) storing its own copies of the SPStyle fields it cares about, and after that redoing SPItem (or relevent subclass) to set them directly via accessors, rather than letting the NRArenaItem copy them from an SPStyle.
Desired end result: NRArenaItem and subclasses no longer need to know anything about SPStyle
-mental
participants (5)
-
unknown＠example.com -
bulia byak -
David Christian Berg -
Jean-François Lemaire -
Peter Moulder