Bug 124999

Summary:

[Texmap] TextureMapperLayer unnecessary object copying

Product:

WebKit

Reporter:

Przemyslaw Szymanski <p.szymanski3>

Component:

Layout and Rendering

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED DUPLICATE

Severity:

Normal

CC:

cmarcelo, commit-queue, fujii.hironori, kondapallykalyan, luiz, noam

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
object copying	none
patch	bfulgham: review-
patch	bfulgham: review-

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	Details Formatted Diff Diff
patch (2.41 KB, patch) 2013-11-29 01:50 PST, Przemyslaw Szymanski	bfulgham: review-	Details Formatted Diff Diff
patch (2.53 KB, patch) 2014-02-20 02:37 PST, Przemyslaw Szymanski	bfulgham: review-	Details Formatted Diff Diff
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.