2008-09-22 Darin Adler <darin@apple.com>
authoroliver@apple.com <oliver@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 23 Sep 2008 07:40:49 +0000 (07:40 +0000)
committeroliver@apple.com <oliver@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 23 Sep 2008 07:40:49 +0000 (07:40 +0000)
- fix https://bugs.webkit.org/show_bug.cgi?id=21008
  getting pixels by index from CanvasPixelArray is unnecessarily slow

Reviewed by Oliver Hunt

* GNUmakefile.am: Added JSCanvasPixelArrayCustom.h.
* WebCore.vcproj/WebCore.vcproj: Ditto.
* WebCore.xcodeproj/project.pbxproj: Ditto.

* bindings/js/JSCanvasPixelArrayCustom.cpp: Removed indexGetter and
  indexSetter. These are now both inlined, so in the header.
* bindings/js/JSCanvasPixelArrayCustom.h: Added. The getByIndex
  function is what's used for HasCustomIndexGetter. Also moved the
  indexSetter function here.

* bindings/scripts/CodeGeneratorJS.pm: Changed HasCustomIndexGetter
  to use a getByIndex member function rather than an indexGetter stat
  member function in a property slot. This lets us avoid the property
  slot mechanism's rule where it turns numeric property names into
  strings in the identifier table, which is good because that's slow.
  Also added a new property CustomHeader that allows IDL files to
  introduce headers to be included -- useful when we have functions
  that we want to inline into the binding.

* html/CanvasPixelArray.idl: Added CustomHeader attribute.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@36801 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebCore/ChangeLog
WebCore/GNUmakefile.am
WebCore/WebCore.vcproj/WebCore.vcproj
WebCore/WebCore.xcodeproj/project.pbxproj
WebCore/bindings/js/JSCanvasPixelArrayCustom.cpp
WebCore/bindings/scripts/CodeGeneratorJS.pm
WebCore/html/CanvasPixelArray.idl

index e0a8c97..d099ebc 100644 (file)
@@ -1,3 +1,31 @@
+2008-09-22  Darin Adler  <darin@apple.com>
+
+        Reviewed by Oliver Hunt.
+
+        - fix https://bugs.webkit.org/show_bug.cgi?id=21008
+          getting pixels by index from CanvasPixelArray is unnecessarily slow
+
+        * GNUmakefile.am: Added JSCanvasPixelArrayCustom.h.
+        * WebCore.vcproj/WebCore.vcproj: Ditto.
+        * WebCore.xcodeproj/project.pbxproj: Ditto.
+
+        * bindings/js/JSCanvasPixelArrayCustom.cpp: Removed indexGetter and
+          indexSetter. These are now both inlined, so in the header.
+        * bindings/js/JSCanvasPixelArrayCustom.h: Added. The getByIndex
+          function is what's used for HasCustomIndexGetter. Also moved the
+          indexSetter function here.
+
+        * bindings/scripts/CodeGeneratorJS.pm: Changed HasCustomIndexGetter
+          to use a getByIndex member function rather than an indexGetter static
+          member function in a property slot. This lets us avoid the property
+          slot mechanism's rule where it turns numeric property names into
+          strings in the identifier table, which is good because that's slow.
+          Also added a new property CustomHeader that allows IDL files to
+          introduce headers to be included -- useful when we have functions
+          that we want to inline into the binding.
+
+        * html/CanvasPixelArray.idl: Added CustomHeader attribute.
+
 2008-09-23  Eric Seidel  <eric@webkit.org>
 
         No review, build fix only.
index 46dc234..be39bc6 100644 (file)
@@ -613,6 +613,7 @@ webcore_sources += \
        WebCore/bindings/js/JSCSSStyleDeclarationCustom.h \
        WebCore/bindings/js/JSCSSValueCustom.cpp \
        WebCore/bindings/js/JSCanvasPixelArrayCustom.cpp \
+       WebCore/bindings/js/JSCanvasPixelArrayCustom.h \
        WebCore/bindings/js/JSCanvasRenderingContext2DCustom.cpp \
        WebCore/bindings/js/JSClipboardCustom.cpp \
        WebCore/bindings/js/JSConsoleCustom.cpp \
index 6df71c4..7ec8379 100644 (file)
                                        >\r
                                </File>\r
                                <File\r
+                                       RelativePath="..\bindings\js\JSCanvasPixelArrayCustom.h"\r
+                                       >\r
+                               </File>\r
+                               <File\r
                                        RelativePath="..\bindings\js\JSCanvasRenderingContext2DCustom.cpp"\r
                                        >\r
                                </File>\r
index 0508523..d8b3c1c 100644 (file)
                9307F1D70AF2D59000DBA31A /* HitTestResult.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 9307F1D50AF2D59000DBA31A /* HitTestResult.cpp */; };
                9307F1D80AF2D59000DBA31A /* HitTestResult.h in Headers */ = {isa = PBXBuildFile; fileRef = 9307F1D60AF2D59000DBA31A /* HitTestResult.h */; settings = {ATTRIBUTES = (Private, ); }; };
                930908910AF7EDE40081DF01 /* HitTestRequest.h in Headers */ = {isa = PBXBuildFile; fileRef = 930908900AF7EDE40081DF01 /* HitTestRequest.h */; };
+               930B3BE20E884921009770C5 /* JSCanvasPixelArrayCustom.h in Headers */ = {isa = PBXBuildFile; fileRef = 930B3BE10E884921009770C5 /* JSCanvasPixelArrayCustom.h */; };
                9326DC0C09DAD5D600AFC847 /* CharsetData.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 656581AC09D14EE6000E61D7 /* CharsetData.cpp */; };
                9327A94209968D1A0068A546 /* HTMLOptionsCollection.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 9327A94109968D1A0068A546 /* HTMLOptionsCollection.cpp */; };
                932871C00B20DEB70049035A /* PlatformMenuDescription.h in Headers */ = {isa = PBXBuildFile; fileRef = 932871BF0B20DEB70049035A /* PlatformMenuDescription.h */; settings = {ATTRIBUTES = (Private, ); }; };
                9307F1D50AF2D59000DBA31A /* HitTestResult.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; path = HitTestResult.cpp; sourceTree = "<group>"; };
                9307F1D60AF2D59000DBA31A /* HitTestResult.h */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.h; path = HitTestResult.h; sourceTree = "<group>"; };
                930908900AF7EDE40081DF01 /* HitTestRequest.h */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.h; path = HitTestRequest.h; sourceTree = "<group>"; };
+               930B3BE10E884921009770C5 /* JSCanvasPixelArrayCustom.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = JSCanvasPixelArrayCustom.h; sourceTree = "<group>"; };
                930CAAD609C495B600229C04 /* CanvasRenderingContext2D.idl */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text; path = CanvasRenderingContext2D.idl; sourceTree = "<group>"; };
                930CAB8809C49EFA00229C04 /* CanvasGradient.idl */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text; path = CanvasGradient.idl; sourceTree = "<group>"; };
                930CAB8F09C49F1B00229C04 /* CanvasPattern.idl */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text; path = CanvasPattern.idl; sourceTree = "<group>"; };
                        children = (
                                BC2ED6BB0C6BD2F000920BFF /* JSAttrCustom.cpp */,
                                BC7951540E15DBFF00A898AB /* JSCanvasPixelArrayCustom.cpp */,
+                               930B3BE10E884921009770C5 /* JSCanvasPixelArrayCustom.h */,
                                1A9EF4560A1B957D00332B63 /* JSCanvasRenderingContext2DCustom.cpp */,
                                BCA83E510D7CE205003421A8 /* JSClipboardCustom.cpp */,
                                C0DFC86F0DB6841A003EAE7C /* JSConsoleCustom.cpp */,
                                B2F34FE60E82F81400F627CD /* DNS.h in Headers */,
                                089582560E857A7E00F82C83 /* ImageLoader.h in Headers */,
                                A8CB413E0E8633FD0032C4F0 /* DashArray.h in Headers */,
+                               930B3BE20E884921009770C5 /* JSCanvasPixelArrayCustom.h in Headers */,
                        );
                        runOnlyForDeploymentPostprocessing = 0;
                };
index 9cd6401..e166053 100644 (file)
  */
 
 #include "config.h"
-#include "JSCanvasPixelArray.h"
-
-#include "CanvasPixelArray.h"
+#include "JSCanvasPixelArrayCustom.h"
 
 using namespace JSC;
 
 namespace WebCore {
 
-JSValue* JSCanvasPixelArray::indexGetter(ExecState* exec, const Identifier& propertyName, const PropertySlot& slot)
-{
-    CanvasPixelArray* array = static_cast<JSCanvasPixelArray*>(slot.slotBase())->impl();
-    unsigned index = slot.index();
-    unsigned char result;
-    if (!array->get(index, result))
-        return jsUndefined();
-    return jsNumber(exec, result);
-}
-
-void JSCanvasPixelArray::indexSetter(ExecState* exec, unsigned index, JSValue* value)
-{
-    double pixelValue = value->toNumber(exec);
-    if (exec->hadException())
-        return;
-    m_impl->set(index, pixelValue); 
-}
-
 JSValue* toJS(ExecState* exec, CanvasPixelArray* pixels)
 {
     if (!pixels)
index 5298d11..a5f365c 100644 (file)
@@ -278,7 +278,7 @@ sub GenerateGetOwnPropertySlotBody
         &$hasNameGetterGeneration();
     }
 
-    my $requiresManualLookup = $dataNode->extendedAttributes->{"HasIndexGetter"} || $dataNode->extendedAttributes->{"HasNameGetter"} || $dataNode->extendedAttributes->{"HasCustomIndexGetter"};
+    my $requiresManualLookup = $dataNode->extendedAttributes->{"HasIndexGetter"} || $dataNode->extendedAttributes->{"HasNameGetter"};
     if ($requiresManualLookup) {
         push(@getOwnPropertySlotImpl, "    const ${namespaceMaybe}HashEntry* entry = ${className}Table.entry(exec, propertyName);\n");
         push(@getOwnPropertySlotImpl, "    if (entry) {\n");
@@ -291,7 +291,11 @@ sub GenerateGetOwnPropertySlotBody
         push(@getOwnPropertySlotImpl, "    bool ok;\n");
         push(@getOwnPropertySlotImpl, "    unsigned index = propertyName.toUInt32(&ok, false);\n");
         push(@getOwnPropertySlotImpl, "    if (ok && index < static_cast<$implClassName*>(impl())->length()) {\n");
-        push(@getOwnPropertySlotImpl, "        slot.setCustomIndex(this, index, indexGetter);\n");
+        if ($dataNode->extendedAttributes->{"HasCustomIndexGetter"}) {
+            push(@getOwnPropertySlotImpl, "        slot.setValue(getByIndex(exec, index));\n");
+        } else {
+            push(@getOwnPropertySlotImpl, "        slot.setCustomIndex(this, index, indexGetter);\n");
+        }
         push(@getOwnPropertySlotImpl, "        return true;\n");
         push(@getOwnPropertySlotImpl, "    }\n");
     }
@@ -413,9 +417,7 @@ sub GenerateHeader
     # Prototype
     push(@headerContent, "    static JSC::JSObject* createPrototype(JSC::ExecState*);\n") if $interfaceName ne "DOMWindow";
 
-    $implIncludes{"${className}Custom.h"} = 1 if
-       $dataNode->extendedAttributes->{"CustomPutFunction"}
-    || $dataNode->extendedAttributes->{"CustomGetOwnPropertySlot"};
+    $implIncludes{"${className}Custom.h"} = 1 if $dataNode->extendedAttributes->{"CustomHeader"} || $dataNode->extendedAttributes->{"CustomPutFunction"};
 
     my $hasGetter = $numAttributes > 0 
                  || $dataNode->extendedAttributes->{"GenerateConstructor"} 
@@ -589,10 +591,13 @@ sub GenerateHeader
     }
 
     # Index getter
-    if ($dataNode->extendedAttributes->{"HasIndexGetter"} || $dataNode->extendedAttributes->{"HasCustomIndexGetter"}) {
+    if ($dataNode->extendedAttributes->{"HasIndexGetter"}) {
         push(@headerContent, "    static JSC::JSValue* indexGetter(JSC::ExecState*, const JSC::Identifier&, const JSC::PropertySlot&);\n");
     }
-
+    if ($dataNode->extendedAttributes->{"HasCustomIndexGetter"}) {
+        push(@headerContent, "    JSC::JSValue* getByIndex(JSC::ExecState*, unsigned index);\n");
+    }
+    
     # Index setter
     if ($dataNode->extendedAttributes->{"HasCustomIndexSetter"}) {
         push(@headerContent, "    void indexSetter(JSC::ExecState*, unsigned index, JSC::JSValue*);\n");
@@ -979,7 +984,11 @@ sub GenerateImplementation
             push(@implContent, "bool ${className}::getOwnPropertySlot(ExecState* exec, unsigned propertyName, PropertySlot& slot)\n");
             push(@implContent, "{\n");
             push(@implContent, "    if (propertyName < static_cast<$implClassName*>(impl())->length()) {\n");
-            push(@implContent, "        slot.setCustomIndex(this, propertyName, indexGetter);\n");
+            if ($dataNode->extendedAttributes->{"HasCustomIndexGetter"}) {
+                push(@implContent, "        slot.setValue(getByIndex(exec, propertyName));\n");
+            } else {
+                push(@implContent, "        slot.setCustomIndex(this, propertyName, indexGetter);\n");
+            }
             push(@implContent, "        return true;\n");
             push(@implContent, "    }\n");
             push(@implContent, "    return getOwnPropertySlot(exec, Identifier::from(exec, propertyName), slot);\n");
index e8f6342..a8a463b 100644 (file)
@@ -29,6 +29,7 @@
 module html {
 
     interface [
+        CustomHeader,
         HasCustomIndexGetter,
         HasCustomIndexSetter,
         GenerateToJS