RESOLVED DUPLICATE of bug 137271 124999
[Texmap] TextureMapperLayer unnecessary object copying 
https://bugs.webkit.org/show_bug.cgi?id=124999
Summary [Texmap] TextureMapperLayer unnecessary object copying
Przemyslaw Szymanski
Reported 2013-11-28 23:05:47 PST
In C++ it is not a good practice to make copy of objects. Mostly because object copying is not cheap operation for CPU especially if an object is large.
Attachments
object copying (2.72 KB, patch)
2013-11-29 00:58 PST, Przemyslaw Szymanski 		no flags
DetailsFormatted DiffDiff
patch (2.41 KB, patch)
2013-11-29 01:50 PST, Przemyslaw Szymanski 		bfulgham: review-
DetailsFormatted DiffDiff
patch (2.53 KB, patch)
2014-02-20 02:37 PST, Przemyslaw Szymanski 		bfulgham: review-
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Przemyslaw Szymanski
Comment 1 2013-11-29 00:58:25 PST
Created attachment 218039 [details] object copying
Przemyslaw Szymanski
Comment 2 2013-11-29 01:47:40 PST
Comment on attachment 218039 [details] object copying wrong attachment format
Przemyslaw Szymanski
Comment 3 2013-11-29 01:50:05 PST
Created attachment 218041 [details] patch
Brent Fulgham
Comment 4 2014-02-19 12:25:18 PST
Comment on attachment 218041 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=218041&action=review > Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:348 > for (size_t i = 0; i < rects.size(); ++i) { Please make this a C++11 loop. > Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:369 > + const IntRect& rect = rects[i]; Ditto.
Przemyslaw Szymanski
Comment 5 2014-02-20 02:37:31 PST
Created attachment 224737 [details] patch
Przemyslaw Szymanski
Comment 6 2014-02-20 02:41:13 PST
(In reply to comment #4) > (From update of attachment 218041 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=218041&action=review > > > Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:348 > > for (size_t i = 0; i < rects.size(); ++i) { > > Please make this a C++11 loop. > > > Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:369 > > + const IntRect& rect = rects[i]; > > Ditto. Thank you for review. Patch is updated. I hope you thought about range-for not std::foreach
Brent Fulgham
Comment 7 2014-04-16 14:09:15 PDT
Comment on attachment 224737 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=224737&action=review Looks very go! Pleaseo switch to 'auto&', but otherwise this looks good. > Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:348 > + for (IntRect& rect : rects) { Please make this for (auto& rect : rects) { > Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:367 > + for (IntRect& rect : rects) { Ditto.
Fujii Hironori
Comment 8 2020-11-08 18:02:53 PST
r174168 fixed this. *** This bug has been marked as a duplicate of bug 137271 ***
Note You need to log in before you can comment on or make changes to this bug.