2015-07-12 13:02 GMT+02:00 Julien Nabet <serval2...@yahoo.fr>: > 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)
Yeap, that's right. The existing comment seems good enough for me. Tamás _______________________________________________ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
