[Libreoffice] [PUSHED] Docuview code cleanup

Hi Matteo,

On Sun, 2011-11-13 at 19:54 +0100, Matteo Casalin wrote:
>      my name's Matteo and this is my first contribution [attempt] to 
> this wonderful piece of work, besides "spreading the word".

        Cool - welcome ! and I'm sorry it took so long to get to reviewing this
properly.

> The attached patch does a little code cleanup in Docuview::DrawSymbol 
> function and its helper, reducing local variables and calls to "real" 
> draw functions.

        :-)

> Please note that:
> * the results of reworked code was not fully tested, since I really
>    don't know were all of those symbols are drawn, but those that I was
>    able to verify look OK to me;

        Great. We see some 'symbols' drawn on buttons often next/previous
buttons that are hidden in various places. Personally I'd prefer to have
alpha transparent, themed bitmaps for all of them but ... ;-)

> * There are still other cleanups that can be done in that code, but I
>    would like to have some feedback before working on them. For example,
>    this patch could include too many changes.

        So, I -think- (and I've inverted some of the senses here) that:

-    if ( nMin & 0x01 )
-        nMin--;
...
-            if ( !(nMin & 0x01) )

        Should be replaced by:

+    const bool bMinSideIsOdd = nMin & 1;
..
+            if ( bMinSideIsOdd )

        Rather than !bMinSideIsOdd, since the nMin-- alters the state ;-) yet
another reason why this unclear & unhelful code needs cleaning up
IMHO :-)

        This code is quite amazing ;-)

            pDev->DrawPixel( Point( nCenterX, nTop ) );
            for ( long i = 1; i <= n2; ++i )
            {
                nTop++;
                pDev->DrawRect( Rectangle (Point( nCenterX-i, nTop ),
                                Point( nCenterX+i, nTop ) ) );
            }

        As a way to draw a triangle for an up-arrow is really quite amazing ...
Particularly when cut/pasted as the down arrow as well. I'd love to see
that stuff made common and replaced with pDev->DrawPolygon or similar
instead :-) cf. tools/inc/tools/gen.hxx and vcl/inc/vcl/outdev.hxx. We
should be able to use Polygon::Rotate() to evaporate lots of this code I
hope, possibly we could even set anti-aliasing transiently to get a
nicer rendered result too :-)

        Anyhow - apart from changing the polarity of the bMinSideIsOdd later in
the code, I've pushed it as is; something so broken deserves all the
fixing it can get ASAP :-)

        Sorry again for the delay; any chance you'd be interested in making
that function fully sane ? :-) it'd be much appreciated.

        Thanks,

                Michael.

-- 
michael.meeks@suse.com  <><, Pseudo Engineer, itinerant idiot