On 12/07/2015 12:47, Zolnai Tamás wrote:
Hi Julien,

2015-07-12 0:44 GMT+02:00 julien2412 <serval2...@yahoo.fr>:
Hello,

Giving a try to tdf#47832, I noticed that there were similar comments in
these files:
GraphicDisplayCacheEntry::IsCacheableAsBitmap:
     487 // This function is based on GraphicManager::ImplCreateOutput(), in
fact it mostly copies
     488 // it, the difference is that this one does not create anything, it
only checks if
     489 // ImplCreateOutput() would use the optimization of using the single
bitmap.
     490 // If you do changes here, change the original function too.
see
http://opengrok.libreoffice.org/xref/core/svtools/source/graphic/grfcache.cxx#487
and GraphicManager::ImplCreateOutput
    1112 // NOTE: If you do changes in this function, check
GraphicDisplayCacheEntry::IsCacheableAsBitmap
    1113 // in grfcache.cxx too.
see
http://opengrok.libreoffice.org/xref/core/svtools/source/graphic/grfmgr2.cxx#1112

But MetaActionType::FONT case isn't managed the same way:
In the first, there's just a fallthrough,
in the second one, there's some treatment.

1) Should we copy/paste the treatment in the first file?
2) Should we remove the treatment in the second file?
3) Should we just tweak the comment?
or simply nothing at all?
The code seems good to me.
GraphicDisplayCacheEntry::IsCacheableAsBitmap() is checks whether the
metafile can be displayed as a single bitmap. In ImplCreateOutput() I
see that MetaActionType::FONT is not handled as a bitmap (see
nNumBitmaps increment), so it's useless to copy that code.
If you check the IsCacheableAsBitmap()'s return value:
     return nNumBitmaps == 1 && !bNonBitmapActionEncountered;
you can see that non of these variables are affected by
MetaActionType::FONT case in ImplCreateOutput().
So I think we don't need any changes here.

Thank you for the explanation Zolnai! :-)
(You didn't mention about tweaking comment so I suppose it doesn't worth it too)

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

Reply via email to