Bug 33205

Summary:	Better Cell Anchoring
Product:	LibreOffice	Reporter:	Caolán McNamara <caolan.mcnamara>
Component:	Calc	Assignee:	Kohei Yoshida <kohei>
Status:	RESOLVED FIXED
Severity:	normal
Priority:	medium
Version:	unspecified
Hardware:	Other
OS:	All
Whiteboard:
Crash report or crash signature:		Regression By:
Attachments:	impl

Description Caolán McNamara 2011-01-17 08:24:05 UTC

Created attachment 42122 [details]
impl

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

http://www.openoffice.org/issues/show_bug.cgi?id=102061
http://www.openoffice.org/issues/show_bug.cgi?id=94991
http://www.openoffice.org/issues/show_bug.cgi?id=115025
http://www.openoffice.org/issues/show_bug.cgi?id=116164
http://www.openoffice.org/issues/show_bug.cgi?id=94028

(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]
impl

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.