Re: [SOLVED] Re: About GraphicDisplayCacheEntry::IsCacheableAsBitmap/GraphicManager::ImplCreateOutput
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
[SOLVED] Re: About GraphicDisplayCacheEntry::IsCacheableAsBitmap/GraphicManager::ImplCreateOutput
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
Re: About GraphicDisplayCacheEntry::IsCacheableAsBitmap/GraphicManager::ImplCreateOutput
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. Best Regards, Tamás ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
