Bug 33205 - Better Cell Anchoring
Summary: Better Cell Anchoring
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
(earliest affected)
Hardware: Other All
: medium normal
Assignee: Kohei Yoshida
Depends on:
Reported: 2011-01-17 08:24 UTC by Caolán McNamara
Modified: 2011-01-19 00:31 UTC (History)
0 users

See Also:
Crash report or crash signature:

impl (99.14 KB, patch)
2011-01-17 08:24 UTC, Caolán McNamara

Note You need to log in before you can comment on or make changes to this bug.
Description Caolán McNamara 2011-01-17 08:24:05 UTC
Created attachment 42122 [details]

This is a patch I've been working on for a while.

Basically see http://www.openoffice.org/issues/show_bug.cgi?id=102061 for some details.

Hiding rows that have cell or page anchored graphics in them often has horrible side effects, especially when a graphic is completely hidden by shrinking the rows and then re-showing the rows the graphic size gets totally mangled.

With this patch in place graphics get restored to their original size and position on hide and show of rows. 

There's a unit test added as part of this patch, so that's probably the place to look at first, the bit that touches qa/unit/ucalc.cxx to see the motivation.

The idea is to basically use the drawing layer to keep track of the position, which allows "the right thing to happen" when the shape is resized manually and when the rows its anchored to are resized etc.

With that in place the rest is then just logical fall out, and chunks of code disappear and become unused.

Related issues are...


(note the last example isn't importing correctly and the moment with this patch in place or not, the objects appear imported twice and without color)
Comment 1 Caolán McNamara 2011-01-17 08:26:10 UTC
I don't know if there are edge cases not covered here, but at least we've now got the infrastructure to lock in fixes for them if they show up
Comment 2 Rainer Bielefeld Retired 2011-01-17 20:55:14 UTC
Comment on attachment 42122 [details]

Marked as patch
Comment 3 Kohei Yoshida 2011-01-18 15:25:37 UTC
Hi Caolan,

So, this is undoubtedly a large patch, and there is no way I can do a meaningful review by examining the patch itself.  The fact that I'm not that familiar with the drawing object handling in Calc yet doesn't help either. ;-)

Having said that, the timing couldn't have been better because I was also looking into reworking Calc's drawing layer, but for a different reason.  My reason was to boost performance of row filtering which currently cannot be as fast as I want it to be because of the way Calc handles drawing objects.  And I'm glad to see that you decided to take the same approach I had mind (i.e. store the anchoring cell positions and the offset within those cells).

I did a quick scan of the patch, and the code change looks good.  For now, I'm willing to let the unit test dictate "pass or fail", commit this patch, and go through the change in detail as I do more testing on master, and make this code more robust as we go.
Comment 4 Kohei Yoshida 2011-01-18 15:59:55 UTC
Ok.  Committed on master.

Some info for QA folks.  This is strictly an internal refactoring, so please look out for any changes in drawing object behavior, but do note that some behaviors will be different because the current behavior is broken in some scenarios.
Comment 5 Caolán McNamara 2011-01-19 00:31:47 UTC
My first cut at this some time ago caused problems with Drawing Objects that aren't "normal" ones, but artefacts of other elements, e.g. PostIts and those arrows and circles drawn to indicate what cells are inputs into calculation targets in other cells (or something like that) They should work fine now as well, but maybe there are other edge cases along those lines somewhere.