[Libreoffice] [PUSHED] More icons removal

Michael Meeks michael.meeks at suse.com
Fri Nov 18 09:15:56 PST 2011


Hi Julien,

On Fri, 2011-11-18 at 08:43 -0800, Julien Chaffraix wrote:
> attached are more icons removals.

	Nice :-)

> I tried them with a clean-build to avoid any building issues and
> removed the reference one by one to make sure I did not miss anything.

	So - just one quick query; if you check:

	vcl/unx/generic/app/soicon.cxx

	you'll see that we calculate the resource id at run-time (this is a bit
of a gotcha):

    if( iconSize >= 48 )
        nIconSizeOffset = SV_ICON_SIZE48_START;
    else if( iconSize >= 32 )
        nIconSizeOffset = SV_ICON_SIZE32_START;
    else if( iconSize >= 16 )
        nIconSizeOffset = SV_ICON_SIZE16_START;
    else
        return sal_False;

    BitmapEx aIcon( ResId(nIconSizeOffset + nIcon, *ImplGetResMgr()));

	Did you consider whether it is possible to generate the same number as
SV_ICON_ID_PRESENTATION_TEMPLATE in some other way ? presumably it is
not ( in a sane world ), and you removed it from the header which looks
good. There can be -some- cases like this where a compile would succeed,
that might fail at runtime - sometimes instead of using ImageLists in a
pleasant way, this sort of arithmetic happens ;-)

	In some ways, I'm surprised to see:

Removed SV_ICON_ID_PRESENTATION_COMPRESSED and all associated code.

	Given that this (apparently) has an 'industrial' version, and we tried
to focus on icons that were actually visible for that, but - it also
seems correct :-) (in general it's worth being a bit more paranoid about
removing them).

	Anyhow - thanks for the test compile from clean, I'm pushing them to
master now.

	Out of interest, how many more unused icons have you detected ? and/or
do you have a nice script for that ? :-)

	Great work !

		Michael.

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



More information about the LibreOffice mailing list