[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