Summary: | [CSS Shaders] Implement normal blend mode and source-atop compositing mode | ||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Max Vujovic <mvujovic> | ||||||||||||||||||||||||||||||||||||
Component: | Layout and Rendering | Assignee: | Max Vujovic <mvujovic> | ||||||||||||||||||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||||||||||||||||||
Severity: | Normal | CC: | achicu, dglazkov, emalasky, eric, jnetterfield, peter+ews, webkit.review.bot | ||||||||||||||||||||||||||||||||||||
Priority: | P2 | ||||||||||||||||||||||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||||||||||||||||||||
Bug Depends on: | |||||||||||||||||||||||||||||||||||||||
Bug Blocks: | 71392, 93870, 94358, 94492, 94725, 94728, 97309 | ||||||||||||||||||||||||||||||||||||||
Attachments: |
|
Description
Max Vujovic
2012-08-13 11:08:04 PDT
Created attachment 158428 [details]
Patch
Alex, could you take a look at this please?
Comment on attachment 158428 [details] Patch Attachment 158428 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13493840 Comment on attachment 158428 [details] Patch Attachment 158428 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/13492747 Comment on attachment 158428 [details] Patch Attachment 158428 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/13489905 Comment on attachment 158428 [details] Patch Attachment 158428 [details] did not pass cr-android-ews (chromium-android): Output: http://queues.webkit.org/results/13512083 Created attachment 159172 [details]
Patch
I've refactored the patch based on a verbal discussion with Alex. The details are in WebCore/ChangeLog in the patch.
Comment on attachment 159172 [details] Patch Attachment 159172 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13530129 New failing tests: css3/filters/custom/custom-filter-color-matrix.html css3/filters/custom/filter-fallback-to-software.html css3/filters/custom/invalid-custom-filter-shader.html css3/filters/custom/custom-filter-composite-source-atop.html Created attachment 159183 [details]
Archive of layout-test-results from gce-cr-linux-03
The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-03 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Created attachment 159237 [details]
Patch
Added test expectations for Chromium for new tests.
Comment on attachment 159237 [details] Patch Attachment 159237 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13529247 New failing tests: css3/filters/custom/custom-filter-color-matrix.html Created attachment 159252 [details]
Archive of layout-test-results from gce-cr-linux-06
The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-06 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Created attachment 159462 [details] Patch Added lines to chromium/TestExpectations requesting Win and Linux baselines for new tests. This is captured in bug 94492. View in context: https://bugs.webkit.org/attachment.cgi?id=159237&action=review Saving first round of this review. Some small improvements. > Source/WebCore/platform/graphics/filters/CustomFilterGlobalContext.cpp:50 > +ANGLEWebKitBridge* CustomFilterGlobalContext::webglValidator() I think naming this webGLShadersValidator() would make it more similar to cssShadersValidator(). I would also welcome an explanation about when we use webgl vs. css shaders. > Source/WebCore/platform/graphics/filters/CustomFilterGlobalContext.cpp:64 > +PassOwnPtr<ANGLEWebKitBridge> CustomFilterGlobalContext::createValidator(ShShaderSpec shaderSpec) And maybe this one would be createShadersValidator? > Source/WebCore/platform/graphics/filters/CustomFilterValidatedProgram.cpp:105 > +bool CustomFilterValidatedProgram::validateMixVertexShader(const String& originalShader, String& validatedShader, String& validationLog) bool CustomFilterValidatedProgram::validateMixVertexShader(const String& originalShader, String& validationLog) { m_validatedVertexShader = ...; } > Source/WebCore/platform/graphics/filters/CustomFilterValidatedProgram.cpp:128 > +bool CustomFilterValidatedProgram::validateMixFragmentShader(const String& originalShader, String& validatedShader, String& validationLog) This is doing more than just validating the shader. > Source/WebCore/platform/graphics/filters/CustomFilterValidatedProgram.cpp:147 > + uniform sampler2D css_u_texture; Can you add a comment about why css_u_texture cannot be used in user code? > Source/WebCore/platform/graphics/filters/FECustomFilter.cpp:-198 > - m_compiledProgram = m_globalContext->getCompiledProgram(m_program->programInfo()); Why not keep the compiled program in this patch. It would remove a lot of changes in this patch. > Source/WebCore/platform/graphics/filters/FECustomFilter.cpp:279 > + CustomFilterCompiledProgram* compiledProgram = m_validatedProgram->compiledProgram().get(); Add an assert here to check that the program is initialized. > Source/WebCore/platform/graphics/filters/FECustomFilter.cpp:322 > + offset += bindVertexAttribute(compiledProgram->positionAttribLocation(), 4, offset); Since you are refactoring this code, what about using some const values for the offsets instead: static const unsigned PositionAttributeOffset = 0; static const unsigned TextureAttributeOffset = PositionAttributeOffset + sizeof(float) * 4; ... > Source/WebCore/rendering/FilterEffectRenderer.cpp:358 > +PassRefPtr<FECustomFilter> FilterEffectRenderer::createCustomFilterEffect(Document* document, CustomFilterOperation* operation) Can this be static? > Source/WebCore/rendering/FilterEffectRenderer.cpp:362 > + return effect; why not just return 0? > Source/WebCore/rendering/FilterEffectRenderer.cpp:366 > + return effect; ditto > Source/WebCore/rendering/FilterEffectRenderer.cpp:372 > + return effect; ditto > Source/WebCore/rendering/FilterEffectRenderer.cpp:374 > + effect = FECustomFilter::create(this, globalContext, validatedProgram, operation->parameters(), This is the first place where you actually need the RefPtr<FECustomFilter> > Source/WebCore/rendering/FilterEffectRenderer.cpp:377 > + m_hasCustomShaderFilter = true; Can you move this back to the caller and keep this function static? > Source/WebCore/rendering/FilterEffectRenderer.h:45 > +#include "FECustomFilter.h" There's no need to include this file header. > Source/WebCore/rendering/FilterEffectRenderer.h:124 > + PassRefPtr<FECustomFilter> createCustomFilterEffect(Document*, CustomFilterOperation*); Keep this as a static function in the file. Created attachment 160038 [details] Patch Thanks for the review, Alex. I've uploaded another patch. (In reply to comment #13) > View in context: https://bugs.webkit.org/attachment.cgi?id=159237&action=review > > Saving first round of this review. Some small improvements. > > > Source/WebCore/platform/graphics/filters/CustomFilterGlobalContext.cpp:50 > > +ANGLEWebKitBridge* CustomFilterGlobalContext::webglValidator() > > I think naming this webGLShadersValidator() would make it more similar to cssShadersValidator(). I would also welcome an explanation about when we use webgl vs. css shaders. I renamed them to webglShaderValidator() and mixShaderValidator(). I call it mixShaderValidator() because it's used to validate a special type of CSS Shader- a shader referenced from a CSS mix function. I've added some comments to describe this distinction. > > > Source/WebCore/platform/graphics/filters/CustomFilterGlobalContext.cpp:64 > > +PassOwnPtr<ANGLEWebKitBridge> CustomFilterGlobalContext::createValidator(ShShaderSpec shaderSpec) > > And maybe this one would be createShadersValidator? I renamed it to createShaderValidator (shader singular, not shaders plural). > > > Source/WebCore/platform/graphics/filters/CustomFilterValidatedProgram.cpp:105 > > +bool CustomFilterValidatedProgram::validateMixVertexShader(const String& originalShader, String& validatedShader, String& validationLog) > > bool CustomFilterValidatedProgram::validateMixVertexShader(const String& originalShader, String& validationLog) { > m_validatedVertexShader = ...; > } Good idea. See the next point below. > > > Source/WebCore/platform/graphics/filters/CustomFilterValidatedProgram.cpp:128 > > +bool CustomFilterValidatedProgram::validateMixFragmentShader(const String& originalShader, String& validatedShader, String& validationLog) > > This is doing more than just validating the shader. I've moved the validation into the CustomFilterValidatedProgram constructor. It selects a validator like this: ANGLEWebKitBridge* validator = programInfo.mixSettings().enabled ? m_globalContext->mixShaderValidator() : m_globalContext->webglShaderValidator(); Then, it performs validation. I've changed these functions to simply: void rewriteMixVertexShader(); void rewriteMixFragmentShader(); > > > Source/WebCore/platform/graphics/filters/CustomFilterValidatedProgram.cpp:147 > > + uniform sampler2D css_u_texture; > > Can you add a comment about why css_u_texture cannot be used in user code? Good idea. Done. > > > Source/WebCore/platform/graphics/filters/FECustomFilter.cpp:-198 > > - m_compiledProgram = m_globalContext->getCompiledProgram(m_program->programInfo()); > > Why not keep the compiled program in this patch. It would remove a lot of changes in this patch. > Done. > > Source/WebCore/platform/graphics/filters/FECustomFilter.cpp:279 > > + CustomFilterCompiledProgram* compiledProgram = m_validatedProgram->compiledProgram().get(); > > Add an assert here to check that the program is initialized. > I've added an assert a little earlier in FECustomFilter::bindProgramAndBuffers, which is the only caller of this method (FECustomFilter::bindProgramParameters). > > Source/WebCore/platform/graphics/filters/FECustomFilter.cpp:322 > > + offset += bindVertexAttribute(compiledProgram->positionAttribLocation(), 4, offset); > > Since you are refactoring this code, what about using some const values for the offsets instead: > static const unsigned PositionAttributeOffset = 0; > static const unsigned TextureAttributeOffset = PositionAttributeOffset + sizeof(float) * 4; > ... Done. I've put the constants in the function for now. Ideally, they should be members of CustomFilterMesh because CustomFilterMesh uses the same values. I've add a FIXME and a created bug 94755. I think this should be done regardless of how we factor this particular function. > > > Source/WebCore/rendering/FilterEffectRenderer.cpp:358 > > +PassRefPtr<FECustomFilter> FilterEffectRenderer::createCustomFilterEffect(Document* document, CustomFilterOperation* operation) > > Can this be static? Done. However, I do have to pass the "this" pointer because FECustomFilter::create takes in the FilterEffectRenderer. I think that's fine though. > > > Source/WebCore/rendering/FilterEffectRenderer.cpp:362 > > + return effect; > > why not just return 0? Done. I like that better. > > > Source/WebCore/rendering/FilterEffectRenderer.cpp:366 > > + return effect; > > ditto Done. > > > Source/WebCore/rendering/FilterEffectRenderer.cpp:372 > > + return effect; > > ditto Done. > > > Source/WebCore/rendering/FilterEffectRenderer.cpp:374 > > + effect = FECustomFilter::create(this, globalContext, validatedProgram, operation->parameters(), > > This is the first place where you actually need the RefPtr<FECustomFilter> Now, I don't make the RefPtr. I just do "return FECustomFilter::create(...);". > > > Source/WebCore/rendering/FilterEffectRenderer.cpp:377 > > + m_hasCustomShaderFilter = true; > > Can you move this back to the caller and keep this function static? Done. > > > Source/WebCore/rendering/FilterEffectRenderer.h:45 > > +#include "FECustomFilter.h" > > There's no need to include this file header. > I need it because of this line in FilterEffectRenderer.h: static PassRefPtr<FECustomFilter> createCustomFilterEffect(Filter*, Document*, CustomFilterOperation*); > > Source/WebCore/rendering/FilterEffectRenderer.h:124 > > + PassRefPtr<FECustomFilter> createCustomFilterEffect(Document*, CustomFilterOperation*); > > Keep this as a static function in the file. Done. Created attachment 160065 [details]
Patch
Rebased patch.
Comment on attachment 160065 [details] Patch Attachment 160065 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13565571 New failing tests: css3/filters/custom/effect-custom.html Created attachment 160076 [details]
Archive of layout-test-results from gce-cr-linux-04
The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-04 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Created attachment 160084 [details]
Patch
EWS was right- my vertex attribute offset calculation was off. Let's try this again.
Created attachment 160277 [details] Patch A few more small changes related to Alex's review: (In reply to comment #14) > > > Source/WebCore/rendering/FilterEffectRenderer.cpp:377 > > > + m_hasCustomShaderFilter = true; > > > > Can you move this back to the caller and keep this function static? > > Done. > Now I made createCustomFilterEffect() a static function in FilterEffectRenderer.cpp. > > > > > Source/WebCore/rendering/FilterEffectRenderer.h:45 > > > +#include "FECustomFilter.h" > > > > There's no need to include this file header. > > > > I need it because of this line in FilterEffectRenderer.h: > static PassRefPtr<FECustomFilter> createCustomFilterEffect(Filter*, Document*, CustomFilterOperation*); Nevermind, you were right. The include is gone now. Comment on attachment 160277 [details]
Patch
Bots are looking good. Setting r?, cq?.
Comment on attachment 160277 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=160277&action=review > LayoutTests/ChangeLog:10 > + Reviewed by NOBODY (OOPS!). > + > + Add a test for normal blend mode mode and source-atop compositing mode using css_MixColor in > + the shader. > + Add a test using css_ColorMatrix in the shader. I think we should add a single line here explaining that we've added these two new attribs/uniforms to the shaders. The description "normal blend mode and source-atop compositing" is probably not enough to someone just looking at this changelog (and not following the bug link). > LayoutTests/css3/filters/custom/custom-filter-composite-source-atop.html:35 > + /* The fragment shader writes a blue square to css_MixColor in the lower right corner of the destination div. */ > + /* It writes transparent elsewhere. */ > + -webkit-filter: custom(url('../resources/pass-tex-coord.vs') mix(url('../resources/composite.fs') normal source-atop)); > + } > + /* Draws a yellow square in the upper left corner of the destination div. */ > + /* The rest of the destination div is transparent. */ This is a cool test! > Source/WebCore/ChangeLog:10 > + Instead of allowing direct texture access in an author's shader via u_texture, CSS > + Shaders blends special symbols in the author's shader (css_MixColor and > + css_ColorMatrix) with the DOM element texture. It's a shame that this makes testing outside the Web more complicated, but that's a spec issue rather than this patch. > Source/WebCore/ChangeLog:21 > + This patch introduces a new class, CustomFilterValidatedProgram. > + CustomFilterValidatedProgram validates the shader using ANGLE. If the shader uses " ... a new class, CustomFilterValidatedProgram, which validates the shader..." > Source/WebCore/ChangeLog:31 > + After validation, CustomFilterValidatedProgram adds blending, compositing, and > + texture access shader code to the author's original shaders. The definitions for > + css_MixColor and css_ColorMatrix are added before the author's fragment shader > + code so that the author code can access them. The blending, compositing, and I assume it is still ok if the author includes them in their shader (as long as they are defined the same way)? I'm thinking of a non-Web test system for just the standalone shaders. > Source/WebCore/ChangeLog:41 > + This is a great ChangeLog. Thankyou! > Source/WebCore/WebCore.gyp/WebCore.gyp:-1618 > - ['exclude', 'platform/graphics/ANGLEWebKitBridge\\.(cpp|h)$'], I assume this won't cause any issues in Chromium? > Source/WebCore/platform/graphics/ANGLEWebKitBridge.h:34 > -#elif !PLATFORM(GTK) && !PLATFORM(EFL) && !PLATFORM(BLACKBERRY) > +#elif !PLATFORM(GTK) && !PLATFORM(EFL) && !PLATFORM(BLACKBERRY) && !PLATFORM(CHROMIUM) This probably answers my question above :) > Source/WebCore/platform/graphics/ANGLEWebKitBridge.h:56 > - bool validateShaderSource(const char* shaderSource, ANGLEShaderType shaderType, String& translatedShaderSource, String& shaderValidationLog, int extraCompileOptions); > + bool validateShaderSource(const char* shaderSource, ANGLEShaderType, String& translatedShaderSource, String& shaderValidationLog, int extraCompileOptions = 0); Thanks. I probably should have done this in my original patch. > Source/WebCore/platform/graphics/filters/CustomFilterCompiledProgram.cpp:132 > + m_internalTexAttribLocation = m_context->getAttribLocation(m_program, "css_a_texCoord"); m_internalTexCoordAttribLocation? > Source/WebCore/platform/graphics/filters/CustomFilterCompiledProgram.h:63 > + int internalTexAttribLocation() const { return m_internalTexAttribLocation; } Similarly, internalTexCoordAttribLocation() ? > Source/WebCore/platform/graphics/filters/CustomFilterGlobalContext.cpp:62 > +ANGLEWebKitBridge* CustomFilterGlobalContext::webglShaderValidator() > +{ > + if (!m_webglShaderValidator) > + m_webglShaderValidator = createShaderValidator(SH_WEBGL_SPEC); > + return m_webglShaderValidator.get(); > +} > + > +ANGLEWebKitBridge* CustomFilterGlobalContext::mixShaderValidator() > +{ > + if (!m_mixShaderValidator) > + m_mixShaderValidator = createShaderValidator(SH_CSS_SHADERS_SPEC); > + return m_mixShaderValidator.get(); > +} Is there any reason why we wouldn't always want to use the SH_CSS_SHADERS_SPEC validator, even if the shader doesn't use mix or the generated attribs/uniforms? > Source/WebCore/platform/graphics/filters/CustomFilterGlobalContext.h:62 > + // CSS shaders referenced from the CSS mix function should be validated slightly differently than WebGL shaders. > + // This ANGLE validator uses the SH_CSS_SHADERS_SPEC flag. > + // Under this flag, most notably: > + // - The "gl_FragColor" built-in is not available. > + // - Instead, the "css_MixColor" and "css_ColorMatrix" built-ins are available. Not really an issue for this patch, but this answers my question above about always using the CSS shader validation. It would be really nice if gl_FragColor was auto-swappable with css_MixColor. Oh well. > Source/WebCore/platform/graphics/filters/CustomFilterValidatedProgram.cpp:91 > + if (!vertexShaderValid || !fragmentShaderValid) > + // FIXME: Report the validation errors. > + // https://bugs.webkit.org/show_bug.cgi?id=74416 > + return; Please add {} since you have multiline comments. > Source/WebCore/platform/graphics/filters/CustomFilterValidatedProgram.h:77 > + // 'detachGlobalContext' is called when the CustomFilterGlobalContext is deleted > + // and there's no need for the callback anymore. You mean 'detachFromGlobalContext' Comment on attachment 160277 [details]
Patch
Oops. Removing cq+ in case Max wants to change things.
Comment on attachment 160277 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=160277&action=review >> LayoutTests/ChangeLog:10 >> + Add a test using css_ColorMatrix in the shader. > > I think we should add a single line here explaining that we've added these two new attribs/uniforms to the shaders. The description "normal blend mode and source-atop compositing" is probably not enough to someone just looking at this changelog (and not following the bug link). Will do. >> LayoutTests/css3/filters/custom/custom-filter-composite-source-atop.html:35 >> + /* The rest of the destination div is transparent. */ > > This is a cool test! Thanks! >> Source/WebCore/ChangeLog:10 >> + css_ColorMatrix) with the DOM element texture. > > It's a shame that this makes testing outside the Web more complicated, but that's a spec issue rather than this patch. Yeah, it is too bad. There's no technical barrier to letting authors use gl_FragColor instead of css_MixColor in mix shaders. We just need some spec changes and ANGLE changes. The original intention for using css_MixColor instead of gl_FragColor was to make it clear that an author is not writing a real pixel value. That particular functionality is intended for the pure GLSL shaders that don't use mix. One day, I hope we can add u_texture access back to the pure GLSL shaders, with the necessary restrictions in place. Then, I'm thinking most people will write non-mix, pure GLSL shaders. At that point, mix shaders will be syntactic sugar for certain types of effects that particularly lend themselves to using a color matrix or blending and compositing. >> Source/WebCore/ChangeLog:21 >> + CustomFilterValidatedProgram validates the shader using ANGLE. If the shader uses > > " ... a new class, CustomFilterValidatedProgram, which validates the shader..." That's better. Thanks. >> Source/WebCore/ChangeLog:31 >> + code so that the author code can access them. The blending, compositing, and > > I assume it is still ok if the author includes them in their shader (as long as they are defined the same way)? I'm thinking of a non-Web test system for just the standalone shaders. It's an error if the author defines css_MixColor or css_ColorMatrix explicitly, just like its an error if the author tries to define gl_FragColor. For example, this would be an error: "vec4 css_MixColor = vec4(1.0);" This is because css_MixColor and css_ColorMatrix are built-ins just like gl_FragColor, not uniforms or attributes. >> Source/WebCore/ChangeLog:41 >> + > > This is a great ChangeLog. Thankyou! Welcome! :) >> Source/WebCore/platform/graphics/ANGLEWebKitBridge.h:34 >> +#elif !PLATFORM(GTK) && !PLATFORM(EFL) && !PLATFORM(BLACKBERRY) && !PLATFORM(CHROMIUM) > > This probably answers my question above :) Yup, I've tried it on Chromium mac. Also, the cr-linux bot seems happy linking Chromium's copy of ANGLE. >> Source/WebCore/platform/graphics/filters/CustomFilterCompiledProgram.cpp:132 >> + m_internalTexAttribLocation = m_context->getAttribLocation(m_program, "css_a_texCoord"); > > m_internalTexCoordAttribLocation? Thanks! Good eyes! >> Source/WebCore/platform/graphics/filters/CustomFilterCompiledProgram.h:63 >> + int internalTexAttribLocation() const { return m_internalTexAttribLocation; } > > Similarly, internalTexCoordAttribLocation() ? Yup. >> Source/WebCore/platform/graphics/filters/CustomFilterGlobalContext.h:62 >> + // - Instead, the "css_MixColor" and "css_ColorMatrix" built-ins are available. > > Not really an issue for this patch, but this answers my question above about always using the CSS shader validation. It would be really nice if gl_FragColor was auto-swappable with css_MixColor. Oh well. By auto-swappable, do you mean if an author writes "gl_FragColor", then we pretend they wrote "css_MixColor" in mix shaders? That is, we make gl_FragColor and css_MixColor synonyms. >> Source/WebCore/platform/graphics/filters/CustomFilterValidatedProgram.cpp:91 >> + return; > > Please add {} since you have multiline comments. Will do. >> Source/WebCore/platform/graphics/filters/CustomFilterValidatedProgram.h:77 >> + // and there's no need for the callback anymore. > > You mean 'detachFromGlobalContext' Yup, thanks! Created attachment 161311 [details]
Patch for Landing
Added the changes from Dean's review. Rebased the patch.
Comment on attachment 161311 [details] Patch for Landing Setting cq-. I collided with http://trac.webkit.org/changeset/127046 Created attachment 161333 [details]
Patch for Landing
Rebased patch again.
Comment on attachment 161333 [details]
Patch for Landing
Setting cq?
Comment on attachment 161333 [details]
Patch for Landing
Setting the CQ+.
Created attachment 161364 [details] Patch for Landing Let's try this again, and I want to make sure cr-linux is green before setting cq?. I had an ASCIILiteral related macro expansion issue while merging the changes from: http://trac.webkit.org/changeset/127062 Created attachment 161410 [details]
Patch for Landing
Rebased again. The previous patch went out of date before cr-linux got to it.
Created attachment 161501 [details]
Patch for Landing
Rebased patch.
My patch depends on this. Is it safe to land now? (In reply to comment #32) > My patch depends on this. Is it safe to land now? Yes- it looks safe. I'll ask Alex to set cq+. Comment on attachment 161501 [details]
Patch for Landing
Added CQ+
Comment on attachment 161501 [details] Patch for Landing Rejecting attachment 161501 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: ram.cpp patching file Source/WebCore/platform/graphics/filters/CustomFilterValidatedProgram.h patching file Source/WebCore/platform/graphics/filters/FECustomFilter.cpp patching file Source/WebCore/platform/graphics/filters/FECustomFilter.h patching file Source/WebCore/rendering/FilterEffectRenderer.cpp patching file Source/WebCore/rendering/FilterEffectRenderer.h Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue Full output: http://queues.webkit.org/results/13682863 Created attachment 161557 [details]
Patch for Landing
Another collision. Merged and rebased patch. Setting cq? because we passed the cr-linux bot.
Comment on attachment 161557 [details] Patch for Landing Clearing flags on attachment: 161557 Committed r127217: <http://trac.webkit.org/changeset/127217> All reviewed patches have been landed. Closing bug. |