On Tuesday 8 March 2005 22:14, David Christian Berg wrote:

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

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

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

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

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:

...

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.

[...] 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.