WEB_AUDIO fails to compile on the Leopard bots due to many 32<->64 casting warnings. http://build.webkit.org/builders/Leopard%20Intel%20Debug%20%28Build%29/builds/40113/steps/compile-webkit/logs/warnings%20%28178%29
Created attachment 109695 [details] Patch
Created attachment 109696 [details] Patch This time, without the changes to project.pbxproj and SVGResourcePattern.cpp
+kbr Thanks Jer, I'm looking through it now. I'm also applying the patch and measuring any performance difference in the DynamicsCompressor. I don't expect to see too much difference but I just want to do a quick sanity check.
I've run a few performance tests and it seems like I'm seeing some performance impact due to switching everything to double precision in ZeroPole.cpp and DynamicsCompressorKernel.cpp. It worries me a bit, so I think it's probably a better idea to uniformly switch both of these over to using "float" everywhere in these places. Sorry, I know I suggested you go the double route, but after having tested the performance... As far as the DoublePoint3D - it probably is fine either way (using double as in your patch or switching everything to float). Performance will not be an issue here. I would go with DoublePoint3D if you think it will be useful to have this used in other parts of WebCore.
Created attachment 109814 [details] Patch Made the changes Chris Rogers suggested, namely operating as much as possible as floats.
Attachment 109814 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/platform/audio/Panner.h:50: The parameter name "model" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/audio/HRTFKernel.h:80: The parameter name "channel" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 2 in 89 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 109814 [details] Patch Attachment 109814 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9958324
Created attachment 109823 [details] Patch
Comment on attachment 109823 [details] Patch Attachment 109823 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9942782
patch seems pretty good so far -- I noticed it fails the cr-linux bots since some small changes are necessary to AudioDestinationChromium.cpp (due to sampleRate changing to float). Could you please change AudioDestinationChromium? In file included from /Volumes/Channel/Source/ChromiumTrunk/src/third_party/WebKit/Source/WebKit/chromium/src/AudioDestinationChromium.cpp:33: src/AudioDestinationChromium.h:52:12: error: virtual function 'sampleRate' has a different return type ('double') than the function it overrides (which has return type 'float') double sampleRate() const { return m_sampleRate; } ^ ../../WebCore/platform/audio/AudioDestination.h:53:19: note: overridden virtual function is here virtual float sampleRate() const = 0; ^ /Volumes/Channel/Source/ChromiumTrunk/src/third_party/WebKit/Source/WebKit/chromium/src/AudioDestinationChromium.cpp:53:48: error: out-of-line definition of 'create' does not match any declaration in 'WebCore::AudioDestination' PassOwnPtr<AudioDestination> AudioDestination::create(AudioSourceProvider& provider, double sampleRate) ^~~~~~ /Volumes/Channel/Source/ChromiumTrunk/src/third_party/WebKit/Source/WebKit/chromium/src/AudioDestinationChromium.cpp:104:26: error: out-of-line definition of 'WebCore::AudioDestination::hardwareSampleRate' differs from the declaration in the return type double AudioDestination::hardwareSampleRate() ^ ../../WebCore/platform/audio/AudioDestination.h:54:18: note: previous declaration is here static float hardwareSampleRate(); ^ 3 errors generated.
Created attachment 109839 [details] Patch
This looks fine to me. There's one layout test "webaudio/audiobuffersource-playbackrate.html" which I'll need to rebaseline after this (I'll do it). The test is audibly identical, but differs in the fringes due to the precision changes introduced in this patch. All audio samples run fine - performance is fine
By the way, can you tell me how I can set my Xcode project to see these types of Leopard 32-bit warnings? I'd rather not bother you guys too much as code is modified and new code added...
(In reply to comment #13) > By the way, can you tell me how I can set my Xcode project to see these types of Leopard 32-bit warnings? I'd rather not bother you guys too much as code is modified and new code added... You need to set the compiler to GCC and build 32-bit. In Xcode 4, select the WebCore project, and choose the WebCore Build Settings panel. The option to choose the compiler is in Build Options / Compiler for C / C++ / Objective-C, and LLVM GCC 4.2 should be an option.
Comment on attachment 109839 [details] Patch rs=me
Committed r96745: <http://trac.webkit.org/changeset/96745>