Bug 37712 - Refactor WebGLArray types to share more code
Summary: Refactor WebGLArray types to share more code
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P3 Normal
Assignee: Kenneth Russell
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-04-16 10:47 PDT by Kenneth Russell
Modified: 2010-05-10 10:53 PDT (History)
6 users (show)

See Also:


Attachments
Patch (56.39 KB, patch)
2010-05-10 09:48 PDT, Kenneth Russell
dglazkov: review+
kbr: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kenneth Russell 2010-04-16 10:47:58 PDT
At the WebKit meeting this past week Darin Adler pointed out that there are code sharing opportunities between the various WebGL<T>Array types. Some of these have been addressed with a recent refactoring to the range checking code, but more refactorings would be possible with a template base class between WebGLArray and the concrete types.
Comment 1 Kenneth Russell 2010-05-10 09:48:43 PDT
Created attachment 55562 [details]
Patch

From the ChangeLog:

Refactored WebGLArray types for maximal code sharing. Added WebGLTypedArrayBase and WebGLIntegralTypedArrayBase template superclasses. Changed overridden virtuals to have minimal visibility (private instead of public). Fixed const correctness of a few methods. There are no changes to the public APIs of the WebGLArray classes; the JavaScript bindings are unchanged.

No new tests; existing tests cover this functionality. Ran all WebGL layout tests in Safari and Chromium on Mac OS X.
Comment 2 Dimitri Glazkov (Google) 2010-05-10 10:34:49 PDT
Comment on attachment 55562 [details]
Patch

WebCore/ChangeLog:80
 +  
It would be great to either annotate each change or just remove this huge laundry list of files from the ChangeLog -- you already have refactoring described.

WebCore/html/canvas/WebGLTypedArrayBase.h:85
 +          return adoptRef(new Subclass(buf, byteOffset, length));
Awesome.
Comment 3 Kenneth Russell 2010-05-10 10:52:02 PDT
(In reply to comment #2)
> (From update of attachment 55562 [details])
> WebCore/ChangeLog:80
>  +  
> It would be great to either annotate each change or just remove this huge laundry list of files from the ChangeLog -- you already have refactoring described.

Thanks for the feedback. I'm reluctant to remove the list of modified files / methods because other recent ChangeLog entries do the same thing, so I'm going to leave this as is.

> WebCore/html/canvas/WebGLTypedArrayBase.h:85
>  +          return adoptRef(new Subclass(buf, byteOffset, length));
> Awesome.

Thanks.
Comment 4 Kenneth Russell 2010-05-10 10:53:47 PDT
Committed r59085: <http://trac.webkit.org/changeset/59085>