[Libreoffice] [PARTIALLY PUSHED] [PATCH] Docuview code cleanup

Noel Power nopower at suse.com
Mon Nov 14 07:46:40 PST 2011


On 13/11/11 18:54, Matteo Casalin wrote:
> Hi everybody,
>     my name's Matteo and this is my first contribution [attempt] to 
> this wonderful piece of work, besides "spreading the word".
welcome !!
> 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;
> * There were some inconsistencies in symbol size evaluation, I chose
>   one approach but it could be not the best or correct one;
Personally I am not a ui or vcl person so I only pushed the final couple 
of hunks relating to fixing the colour selection. But you seem to have 
done a great job getting into drawing stuff ( as you can see that code 
needs quite some love ). From what I can see both looking at the code 
and what I could find in the running system with the patch applied ( and 
you mention it yourself ) the size some of the symbols is quite 
different, for example both the SYMBOL_ARROW_XXX & SYMBOL_SPIN_XXX ones 
are noticeably bigger/thicker. To my untrained eye that makes at least 
the scrollbar symbols uglier as they look even more off centre than they 
previously were ( as the arrow head is now nearer the 3d shadow ) but... 
my opinion here is as I said highly dubious. I cc Michael who hopefully 
might know who to help with this
> * 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.
very true and this is a wise approach I think.

Thanks for you contribution, its looks really worthwhile, I am sure 
someone with knowledge in this area will attend this patch very soon.

Noel


More information about the LibreOffice mailing list