Re: [REVIEW][3-5] Re: [[REVIEW][3-5-2] fdo#47717, fdo#45562 sw: yet more border painting regressions

2012-04-20 Thread Michael Stahl
On 20/04/12 14:21, Caolán McNamara wrote:
> On Mon, 2012-04-16 at 17:05 +0200, Michael Stahl wrote:
>> i think i've fixed this problem now (and attached your bugdoc,
>> slightly enhanced, to fdo#45562 as well), so please consider this for
>> libreoffice-3-5, which basically results in painting the borders in
>> almost exactly the same places as the old OOo code, and thus looks safe
>> to me wrt. layering:
>>
>> http://cgit.freedesktop.org/libreoffice/core/commit/?id=5913506b2193e93ca2767ab7365ab2e76ed7848f
> 
> Hmm, doesn't apply against 3-5. Is there now a specific sequence of
> patches required against 3-5, or just the above needing to be tweaked ?

ah yes, it requires the patch from the grandparent mail as well, i.e.

http://cgit.freedesktop.org/libreoffice/core/commit/?id=1024c172a5bfb3d85a86fcf7a046aa2b03950edd

which is also required for the other border line fix to apply (the one
that merges consecutive lines)

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [REVIEW][3-5] Re: [[REVIEW][3-5-2] fdo#47717, fdo#45562 sw: yet more border painting regressions

2012-04-20 Thread Caolán McNamara
On Mon, 2012-04-16 at 17:05 +0200, Michael Stahl wrote:
> i think i've fixed this problem now (and attached your bugdoc,
> slightly enhanced, to fdo#45562 as well), so please consider this for
> libreoffice-3-5, which basically results in painting the borders in
> almost exactly the same places as the old OOo code, and thus looks safe
> to me wrt. layering:
> 
> http://cgit.freedesktop.org/libreoffice/core/commit/?id=5913506b2193e93ca2767ab7365ab2e76ed7848f

Hmm, doesn't apply against 3-5. Is there now a specific sequence of
patches required against 3-5, or just the above needing to be tweaked ?

C.



___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


[REVIEW][3-5] Re: [[REVIEW][3-5-2] fdo#47717, fdo#45562 sw: yet more border painting regressions

2012-04-16 Thread Michael Stahl
On 26/03/12 11:42, Cedric Bosdonnat wrote:
> Hi Michael,
> 
> On Fri, 2012-03-23 at 17:44 +0100, Michael Stahl wrote:
>> On 20/03/12 12:19, Cedric Bosdonnat wrote:
>>> On Fri, 2012-03-16 at 23:29 +0100, Michael Stahl wrote:
 this fix introduces a new array to store the borders and paints them
 after the subsidiary lines are done, effectively on top of the
 subsidiary lines.

 http://cgit.freedesktop.org/libreoffice/core/commit/?id=804d0a896731629397c5328c13c04a45bc55f459
>>>
>>> Thanks for the patch. I apologize as I should have done that a lot
>>> earlier. I cherry-picked and pushed it to -3-5.
>>
>> unfortunately it turns out that the patch introduced a regression,
>> fdo#47717, which is hopefully fixed with this one (as is fdo#45562,
>> which is about borders vs. hellish drawing objects), so please consider
>> it for libreoffice-3-5-2:
>>
>> http://cgit.freedesktop.org/libreoffice/core/commit/?id=1024c172a5bfb3d85a86fcf7a046aa2b03950edd
>>
>> because up until a week ago my knowledge of Writer's drawing code was
>> precisely zero, it would be a good idea to test this a bit.  in case
>> something is still wrong i'd strongly consider reverting the original
>> commit (0f0896c26fb260d1bbf31d7a886df3f61837f0f2).
> 
> The patch fixes the mentioned bugs, but I still found a buggy case. See
> this document:
> http://people.freedesktop.org/~cbosdo/frame-to-back.odt

thanks, i think i've fixed this problem now (and attached your bugdoc,
slightly enhanced, to fdo#45562 as well), so please consider this for
libreoffice-3-5, which basically results in painting the borders in
almost exactly the same places as the old OOo code, and thus looks safe
to me wrt. layering:

http://cgit.freedesktop.org/libreoffice/core/commit/?id=5913506b2193e93ca2767ab7365ab2e76ed7848f

there is also some old border painting code in SwFlyFrmFmt::MakeGraphic,
which looks unnecessary to me because the borders are painted by
SwFlyFrm::Paint already; the patch also happens to fix the following
obscure regression introduced by my earlier patches:

1. create new writer document
2. Insert->Frame
3. OK
4. Edit->Image Map
5. crash

> IMHO, the best effort we could do on that border / subsidiary lines
> painting is to avoid buffering any line / subline. I'm still unsure if
> it's possible at all without having weird corner cases. I still believe
> in "each objects paints itself completely in one shot" idea, but that
> would mean to remove all the subsidiary lines code and consider those as
> the normal lines.
> 
> Any thought on that? I we go in that direction, I'ld revert the patch as
> you mention in 3.5.2 and target that work for 3.6: it's still a bit
> risky and will need heavy testing.

i don't believe this is possible, as there is code that combines
adjacent borders if there is a sufficiently small gap between them (see
SwLineRect::MakeUnion), which is now mostly unused since
0f0896c26fb260d1bbf31d7a886df3f61837f0f2, and results in regressions
such as fdo#38215, about which i'll send another mail.

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice