Re: About GraphicDisplayCacheEntry::IsCacheableAsBitmap/GraphicManager::ImplCreateOutput

Hi Julien,

2015-07-12 0:44 GMT+02:00 julien2412 <serval2412@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.

Best Regards,
Tamás