[SOLVED] Re: About GraphicDisplayCacheEntry::IsCacheableAsBitmap/GraphicManager::ImplCreateOutput

[Julien Nabet]

On 12/07/2015 12:47, Zolnai Tamás wrote:
> Hi Julien,
>
> 2015-07-12 0:44 GMT+02:00 julien2412 <serval2412 at 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