WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED WONTFIX
Bug 102833
[EFL][WK2] 3D pixel tests are failing
https://bugs.webkit.org/show_bug.cgi?id=102833
Summary
[EFL][WK2] 3D pixel tests are failing
Yael
Reported
2012-11-20 13:48:03 PST
In the current setup, we dump pixel results from the web process, bypassing the Texture Mapper. In order to correctly dump the pixels for 3D transforms we have to do the painting in the UI process, the same as was done for Qt. A patch is coming.
Attachments
Patch
(17.76 KB, patch)
2012-11-21 13:33 PST
,
Yael
no flags
Details
Formatted Diff
Diff
Patch
(18.18 KB, patch)
2012-11-25 18:32 PST
,
Yael
eflews.bot
: commit-queue-
Details
Formatted Diff
Diff
Patch
(18.74 KB, patch)
2012-11-25 18:45 PST
,
Yael
no flags
Details
Formatted Diff
Diff
Patch
(18.06 KB, patch)
2012-11-25 18:51 PST
,
Yael
no flags
Details
Formatted Diff
Diff
Patch
(17.74 KB, patch)
2012-11-26 14:27 PST
,
Yael
no flags
Details
Formatted Diff
Diff
Patch
(17.87 KB, patch)
2012-11-26 16:55 PST
,
Yael
no flags
Details
Formatted Diff
Diff
Patch
(17.88 KB, patch)
2012-11-27 08:52 PST
,
Yael
no flags
Details
Formatted Diff
Diff
Patch
(15.08 KB, patch)
2012-11-27 12:42 PST
,
Yael
no flags
Details
Formatted Diff
Diff
Patch
(13.12 KB, patch)
2012-11-27 14:11 PST
,
Yael
no flags
Details
Formatted Diff
Diff
Patch
(14.12 KB, patch)
2012-11-27 15:23 PST
,
Yael
no flags
Details
Formatted Diff
Diff
Show Obsolete
(9)
View All
Add attachment
proposed patch, testcase, etc.
Yael
Comment 1
2012-11-21 13:33:19 PST
Created
attachment 175510
[details]
Patch Generate a snapshot of the view in the UI process instead of the web process. We have to use Texture Mapper in order to correctly paint 3D transforms etc. Since I could not find an efl based API that will give me a snapshot, and I cannot use both evas and openGl API in the same file, I created a new file, in which I use openGL to get a snapshot.
Kenneth Rohde Christiansen
Comment 2
2012-11-21 13:58:07 PST
Comment on
attachment 175510
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=175510&action=review
> Source/WebKit2/UIProcess/API/C/efl/WKView.cpp:45 > + return viewImpl->getSnapshot();
takeShapshot?
> Source/WebKit2/UIProcess/API/efl/EwkViewImpl.cpp:978 > + }
Is this right with those ifdefs?
Yael
Comment 3
2012-11-21 14:02:47 PST
(In reply to
comment #2
)
> (From update of
attachment 175510
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=175510&action=review
> > > Source/WebKit2/UIProcess/API/C/efl/WKView.cpp:45 > > + return viewImpl->getSnapshot(); > > takeShapshot? >
ok
> > Source/WebKit2/UIProcess/API/efl/EwkViewImpl.cpp:978 > > + } > > Is this right with those ifdefs?
yes. both '{' and '}' are inside #if USE(ACCELERATED_COMPOSITING)
Viatcheslav Ostapenko
Comment 4
2012-11-21 14:12:43 PST
Comment on
attachment 175510
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=175510&action=review
> Source/WebKit2/UIProcess/API/efl/SnapshotGL.cpp:24 > +#include <GL/gl.h>
Unfortunately this will not work for opengl ES. For GL ES2 it will be GLES2/gl2.h, for GL ES1 it will be GLES/gl.h I think there should be single place for this, may be OpenGLShims.h ?
Yael
Comment 5
2012-11-21 18:02:25 PST
(In reply to
comment #4
)
> (From update of
attachment 175510
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=175510&action=review
> > > Source/WebKit2/UIProcess/API/efl/SnapshotGL.cpp:24 > > +#include <GL/gl.h> > > Unfortunately this will not work for opengl ES. > For GL ES2 it will be GLES2/gl2.h, for GL ES1 it will be GLES/gl.h > > I think there should be single place for this, may be OpenGLShims.h ?
I got a tip from Carsten Haitzler on the enlightement mailing list about some ecore_x API that I can use. I will give that a try first.
Yael
Comment 6
2012-11-25 18:32:02 PST
Created
attachment 175907
[details]
Patch Address
comment #2
and
comment #4
. XShmGetImage is failing to grab the image, so I resorted to glReadPixels, which is what the Qt port does.
WebKit Review Bot
Comment 7
2012-11-25 18:34:48 PST
Attachment 175907
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1 Source/WebKit2/UIProcess/API/efl/EwkViewImpl.cpp:64: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 13 files If any of these errors are false positives, please file a bug against check-webkit-style.
EFL EWS Bot
Comment 8
2012-11-25 18:36:36 PST
Comment on
attachment 175907
[details]
Patch
Attachment 175907
[details]
did not pass efl-ews (efl): Output:
http://queues.webkit.org/results/14981481
Yael
Comment 9
2012-11-25 18:45:32 PST
Created
attachment 175910
[details]
Patch Forgot to remove one of the X11 headers. sorry :(
WebKit Review Bot
Comment 10
2012-11-25 18:49:40 PST
Attachment 175910
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1 Source/WebKit2/UIProcess/CoordinatedGraphics/CoordinatedBackingStore.cpp:122: One space before end of line comments [whitespace/comments] [5] Source/WebKit2/UIProcess/CoordinatedGraphics/CoordinatedBackingStore.cpp:122: Should have a space between // and comment [whitespace/comments] [4] Total errors found: 2 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
Yael
Comment 11
2012-11-25 18:51:40 PST
Created
attachment 175911
[details]
Patch
Chris Dumez
Comment 12
2012-11-25 21:54:13 PST
Comment on
attachment 175911
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=175911&action=review
> Source/WebKit2/UIProcess/API/efl/EwkViewImpl.cpp:1034 > + delete buffer;
delete[] buffer ?
> Source/WebKit2/UIProcess/API/efl/SnapshotGL.cpp:34 > + unsigned char* buffer = new unsigned char[width * height * 4];
allocated with operator new[].
> Source/WebKit2/UIProcess/API/efl/SnapshotGL.cpp:39 > + unsigned* buf = (unsigned*)buffer;
Please void C style cast
> Source/WebKit2/UIProcess/API/efl/SnapshotGL.cpp:41 > + for (int i = 0; i < height / 2; i++) {
we usually prefer preincrement when possible
> Source/WebKit2/UIProcess/API/efl/SnapshotGL.cpp:42 > + for (int j = 0; j < width; j++) {
ditto
> Tools/WebKitTestRunner/efl/PlatformWebViewEfl.cpp:138 > + RefPtr<cairo_surface_t> surface = WKImageCreateCairoSurface(WKViewGetSnapshot(toAPI(m_view)));
You need to adopt.
> Tools/WebKitTestRunner/efl/PlatformWebViewEfl.cpp:147 > + return adoptWK(WKImageCreateFromCairoSurface(surface.get(), options));
I did not understand why we converted to a cairo surface and then back to a WKImageRef. Why not return WKViewGetSnapshot(toAPI(m_view)) directly?
> Tools/WebKitTestRunner/efl/TestInvocationEfl.cpp:85 > + cairo_t* context = cairo_create(surface);
Smart pointer?
> Tools/WebKitTestRunner/efl/TestInvocationEfl.cpp:121 > + cairo_surface_t* surface = 0;
Smart pointer?
Yael
Comment 13
2012-11-26 05:45:33 PST
(In reply to
comment #12
) Thank you for your comments :)
> (From update of
attachment 175911
[details]
) > we usually prefer preincrement when possible >
Can you please tell me which coding guideline says this?
> I did not understand why we converted to a cairo surface and then back to a WKImageRef. Why not return WKViewGetSnapshot(toAPI(m_view)) directly?
I didn't know if it is ok to return a cairo type in our API. If that is ok, then there is no need for the multiple conversions.
Chris Dumez
Comment 14
2012-11-26 05:58:47 PST
Comment on
attachment 175911
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=175911&action=review
>> Source/WebKit2/UIProcess/API/efl/SnapshotGL.cpp:41 >> + for (int i = 0; i < height / 2; i++) { > > we usually prefer preincrement when possible
It is not documented in the WebKit style guidelines document but Benjamin Poulain commented about this about on one of my patches:
https://bugs.webkit.org/show_bug.cgi?id=101257#c22
I also landed a cleanup patch regarding this in:
https://bugs.webkit.org/show_bug.cgi?id=101859
While this makes no difference for primitive types, it usually does for complex types. Therefore, it is good practice to use prefix operator whenever possible.
>> Tools/WebKitTestRunner/efl/PlatformWebViewEfl.cpp:147 >> + return adoptWK(WKImageCreateFromCairoSurface(surface.get(), options)); > > I did not understand why we converted to a cairo surface and then back to a WKImageRef. Why not return WKViewGetSnapshot(toAPI(m_view)) directly?
What I mean is WKViewGetSnapshot() returns a WKImageRef and this function returns a WKImageRef. Why not return the output of WKViewGetSnapshot() directly? Instead you are creating a surface using WKImageCreateCairoSurface() from the WKImage, then converting the surface back to a WKImage. I did not understand the reason for this.
Yael
Comment 15
2012-11-26 06:12:24 PST
(In reply to
comment #12
)
> (From update of
attachment 175911
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=175911&action=review
> > > Tools/WebKitTestRunner/efl/TestInvocationEfl.cpp:85 > > + cairo_t* context = cairo_create(surface); > > Smart pointer? > > > Tools/WebKitTestRunner/efl/TestInvocationEfl.cpp:121 > > + cairo_surface_t* surface = 0; > > Smart pointer?
TestInvocationEfl.cpp is almost a copy of TestInvocationCairo.cpp, with some modifications specific to EFL. I was hoping to get some comments regarding if people prefer that the file is split, or to add the EFL specific bits to TestInvocationCairo.cpp. I think your comments are result of copy/pasted code from TestInvocationCairo.cpp.
Chris Dumez
Comment 16
2012-11-26 06:16:43 PST
Adding a few GTK guys to see if they have an opinion regarding the addition of EFL specific code in TestInvocationCairo.cpp (
https://bugs.webkit.org/show_bug.cgi?id=102833#c15
).
Yael
Comment 17
2012-11-26 14:27:14 PST
Created
attachment 176066
[details]
Patch Address
comment #12
.
Kenneth Rohde Christiansen
Comment 18
2012-11-26 15:12:22 PST
Comment on
attachment 176066
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=176066&action=review
> Source/WebKit2/UIProcess/API/efl/EwkViewImpl.cpp:1035 > + delete[] buffer; > + return image;
I prefer newline before return
> Source/WebKit2/UIProcess/API/efl/SnapshotGL.cpp:37 > + // Now flip it
Bad comment and ends without punctuation mark
> Source/WebKit2/UIProcess/API/efl/SnapshotGL.h:21 > +#ifndef SnapshotGL_h > +#define SnapshotGL_h
what about SnapshotImageGL ?
Yael
Comment 19
2012-11-26 16:55:52 PST
Created
attachment 176113
[details]
Patch Address
comment #18
.
Gyuyoung Kim
Comment 20
2012-11-26 20:07:17 PST
Comment on
attachment 176113
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=176113&action=review
> Source/WebKit2/PlatformEfl.cmake:45 > + UIProcess/API/efl/SnapshotImageGL.cpp
Wrong alphabetical order.
> Source/WebKit2/UIProcess/API/efl/EwkViewImpl.cpp:1026 > + return WKImageCreateFromCairoSurface(surface.get(), 0);
If you don't use local variable, I think you can remove "{", "}" in this function. return WKImageCreateFromCairoSurface(createSurfaceForImage(sd->image).get(), 0); Or, because you define *surface* variable two places, I think you can declare it first on 1022 line.
> Source/WebKit2/UIProcess/API/efl/EwkViewImpl.cpp:1030 > + Evas_Native_Surface* ns = evas_object_image_native_surface_get(sd->image);
I prefer to use full name. e.g) naviveSurface.
> Source/WebKit2/UIProcess/API/efl/SnapshotImageGL.cpp:21 > +#if USE(ACCELERATED_COMPOSITING)
IMO, it would be better if you move this macro to above #if USE(OPENGL_ES_2)
Yael
Comment 21
2012-11-27 08:52:47 PST
Created
attachment 176279
[details]
Patch Address
comment #20
.
Chris Dumez
Comment 22
2012-11-27 10:30:40 PST
Comment on
attachment 176279
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=176279&action=review
> Tools/WebKitTestRunner/efl/TestInvocationEfl.cpp:45 > +void computeMD5HashStringForCairoSurface(cairo_surface_t* surface, char hashString[33])
There seems to be quite a bit of duplication with TestInvocationCairo.cpp. I really wish we could avoid that. Personally, I would add #ifdefs in TestInvocationCairo.cpp to git EFL port's need. This would avoid the duplication. Any other opinion on this?
Kenneth Rohde Christiansen
Comment 23
2012-11-27 10:49:16 PST
(In reply to
comment #22
)
> (From update of
attachment 176279
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=176279&action=review
> > > Tools/WebKitTestRunner/efl/TestInvocationEfl.cpp:45 > > +void computeMD5HashStringForCairoSurface(cairo_surface_t* surface, char hashString[33]) > > There seems to be quite a bit of duplication with TestInvocationCairo.cpp. I really wish we could avoid that. > > Personally, I would add #ifdefs in TestInvocationCairo.cpp to git EFL port's need. This would avoid the duplication. > Any other opinion on this?
That sounds like the proper thing to do indeed
Yael
Comment 24
2012-11-27 12:42:13 PST
Created
attachment 176329
[details]
Patch Removed TestInvocationEFL.cpp
Yael
Comment 25
2012-11-27 13:55:33 PST
Looks like I cannot include WebCore headers in webkit-gtk, so I cannot use RefCairoPtr in common code. Tools/WebKitTestRunner/cairo/Programs_WebKitTestRunner-TestInvocationCairo.o:TestInvocationCairo.cpp:function WTR::TestInvocation::dumpPixelsAndCompareWithExpected(OpaqueWKImage const*, OpaqueWKArray const*): error: undefined reference to 'void WTF::derefIfNotNull<_cairo_surface>(_cairo_surface*)' Tools/WebKitTestRunner/cairo/Programs_WebKitTestRunner-TestInvocationCairo.o:TestInvocationCairo.cpp:function WTR::TestInvocation::dumpPixelsAndCompareWithExpected(OpaqueWKImage const*, OpaqueWKArray const*): error: undefined reference to 'void WTF::derefIfNotNull<_cairo>(_cairo*)' Tools/WebKitTestRunner/cairo/Programs_WebKitTestRunner-TestInvocationCairo.o:TestInvocationCairo.cpp:function WTR::TestInvocation::dumpPixelsAndCompareWithExpected(OpaqueWKImage const*, OpaqueWKArray const*): error: undefined reference to 'void WTF::derefIfNotNull<_cairo>(_cairo*)' Tools/WebKitTestRunner/cairo/Programs_WebKitTestRunner-TestInvocationCairo.o:TestInvocationCairo.cpp:function WTR::TestInvocation::dumpPixelsAndCompareWithExpected(OpaqueWKImage const*, OpaqueWKArray const*): error: undefined reference to 'void WTF::derefIfNotNull<_cairo_surface>(_cairo_surface*)' collect2: error: ld returned 1 exit status make[1]: *** [Programs/WebKitTestRunner] Error 1 make[1]: Leaving directory `/home/phil/gst/jhbuild/makes/WebKit/WebKitBuild/Release' make: *** [all] Error 2 Failed to build WebKit using 'make'!
Chris Dumez
Comment 26
2012-11-27 14:08:23 PST
(In reply to
comment #25
)
> Looks like I cannot include WebCore headers in webkit-gtk, so I cannot use RefCairoPtr in common code. > > Tools/WebKitTestRunner/cairo/Programs_WebKitTestRunner-TestInvocationCairo.o:TestInvocationCairo.cpp:function WTR::TestInvocation::dumpPixelsAndCompareWithExpected(OpaqueWKImage const*, OpaqueWKArray const*): error: undefined reference to 'void WTF::derefIfNotNull<_cairo_surface>(_cairo_surface*)' > Tools/WebKitTestRunner/cairo/Programs_WebKitTestRunner-TestInvocationCairo.o:TestInvocationCairo.cpp:function WTR::TestInvocation::dumpPixelsAndCompareWithExpected(OpaqueWKImage const*, OpaqueWKArray const*): error: undefined reference to 'void WTF::derefIfNotNull<_cairo>(_cairo*)' > Tools/WebKitTestRunner/cairo/Programs_WebKitTestRunner-TestInvocationCairo.o:TestInvocationCairo.cpp:function WTR::TestInvocation::dumpPixelsAndCompareWithExpected(OpaqueWKImage const*, OpaqueWKArray const*): error: undefined reference to 'void WTF::derefIfNotNull<_cairo>(_cairo*)' > Tools/WebKitTestRunner/cairo/Programs_WebKitTestRunner-TestInvocationCairo.o:TestInvocationCairo.cpp:function WTR::TestInvocation::dumpPixelsAndCompareWithExpected(OpaqueWKImage const*, OpaqueWKArray const*): error: undefined reference to 'void WTF::derefIfNotNull<_cairo_surface>(_cairo_surface*)' > collect2: error: ld returned 1 exit status > make[1]: *** [Programs/WebKitTestRunner] Error 1 > make[1]: Leaving directory `/home/phil/gst/jhbuild/makes/WebKit/WebKitBuild/Release' > make: *** [all] Error 2 > > Failed to build WebKit using 'make'!
This would probably fix it: diff --git a/Tools/WebKitTestRunner/GNUmakefile.am b/Tools/WebKitTestRunner/GNUmakefile.am index 3706003..b1dfb98 100644 --- a/Tools/WebKitTestRunner/GNUmakefile.am +++ b/Tools/WebKitTestRunner/GNUmakefile.am @@ -52,6 +52,7 @@ Programs_WebKitTestRunner_CFLAGS = $(global_cflags) Programs_WebKitTestRunner_LDADD = \ libjavascriptcoregtk-@WEBKITGTK_API_MAJOR_VERSION@.@WEBKITGTK_API_MINOR_VERSION@.la \ + libWebCoreGtk.la \ libwebkit2gtk-@WEBKITGTK_API_MAJOR_VERSION@.@WEBKITGTK_API_MINOR_VERSION@.la \ $(GLOBALDEPS_LIBS) \ $(CAIRO_LIBS) \ Unless there is a specific reason not to have WebKitTestRunner link against WebCore library?
Yael
Comment 27
2012-11-27 14:11:05 PST
Created
attachment 176336
[details]
Patch Removed smart pointer usage from TestInvocationCairo.cpp. They break the GTK build.
Kenneth Rohde Christiansen
Comment 28
2012-11-27 14:40:19 PST
Comment on
attachment 176336
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=176336&action=review
> Source/WebKit2/UIProcess/API/efl/EwkViewImpl.cpp:1026 > + return WKImageCreateFromCairoSurface(createSurfaceForImage(sd->image).get(), 0); > +#if USE(ACCELERATED_COMPOSITING)
newline after return please
> Source/WebKit2/UIProcess/API/efl/SnapshotImageGL.cpp:6 > +/* > + Copyright (C) 2012 Intel Corporation. All rights reserved. > + > + This library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Library General Public > + License as published by the Free Software Foundation; either
why not * ? and why not BSD license? I think we prefer that
> Tools/WebKitTestRunner/cairo/TestInvocationCairo.cpp:110 > +void TestInvocation::forceRepaintDoneCallback(WKErrorRef, void *context)
didFinishRepaint would be a nicer name
> Tools/WebKitTestRunner/cairo/TestInvocationCairo.cpp:112 > + static_cast<TestInvocation*>(context)->m_gotRepaint = true;
gotRepaint is a terrible name. why not m_didRepaint
> Tools/WebKitTestRunner/cairo/TestInvocationCairo.cpp:136 > + > + if (m_gotRepaint) > + surface = WKImageCreateCairoSurface(TestController::shared().mainWebView()->windowSnapshotImage().get()); > + else { > + m_error = true; > + m_errorMessage = "Timed out waiting for repaint\n"; > + m_webProcessIsUnrensponsive = true; > + return; > + }
no. if (!m_didRepaint) { ... return; } cairo_surface_t surface = WKImageCreateCairoSurface(TestController::shared().mainWebView()->windowSnapshotImage().get());
> Tools/WebKitTestRunner/efl/PlatformWebViewEfl.cpp:132 > + Ecore_Evas* ee = ecore_evas_ecore_evas_get(evas_object_evas_get(m_view)); > + > + ASSERT(ee);
no newline before ASSERT, it belong to ee
Yael
Comment 29
2012-11-27 15:00:49 PST
(In reply to
comment #28
)
> (From update of
attachment 176336
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=176336&action=review
> > > Source/WebKit2/UIProcess/API/efl/EwkViewImpl.cpp:1026 > > + return WKImageCreateFromCairoSurface(createSurfaceForImage(sd->image).get(), 0); > > +#if USE(ACCELERATED_COMPOSITING) > > newline after return please >
ok
> > Source/WebKit2/UIProcess/API/efl/SnapshotImageGL.cpp:6 > > +/* > > + Copyright (C) 2012 Intel Corporation. All rights reserved. > > + > > + This library is free software; you can redistribute it and/or > > + modify it under the terms of the GNU Library General Public > > + License as published by the Free Software Foundation; either > > why not * ? and why not BSD license? I think we prefer that >
I copied the license of EwkViewImpl, I'll change that.
> > Tools/WebKitTestRunner/cairo/TestInvocationCairo.cpp:110 > > +void TestInvocation::forceRepaintDoneCallback(WKErrorRef, void *context) > > didFinishRepaint would be a nicer name >
The functions names are not introduced in this patch. I can rename them as a followup
> > Tools/WebKitTestRunner/cairo/TestInvocationCairo.cpp:112 > > + static_cast<TestInvocation*>(context)->m_gotRepaint = true; > > gotRepaint is a terrible name. why not m_didRepaint >
copy/paste from Qt :) I'll rename
> > Tools/WebKitTestRunner/cairo/TestInvocationCairo.cpp:136 > > + > > + if (m_gotRepaint) > > + surface = WKImageCreateCairoSurface(TestController::shared().mainWebView()->windowSnapshotImage().get()); > > + else { > > + m_error = true; > > + m_errorMessage = "Timed out waiting for repaint\n"; > > + m_webProcessIsUnrensponsive = true; > > + return; > > + } > > no. > > if (!m_didRepaint) { > ... > return; > } >
same as before, copy/paste from Qt :) Will change.
> cairo_surface_t surface = WKImageCreateCairoSurface(TestController::shared().mainWebView()->windowSnapshotImage().get()); > > > Tools/WebKitTestRunner/efl/PlatformWebViewEfl.cpp:132 > > + Ecore_Evas* ee = ecore_evas_ecore_evas_get(evas_object_evas_get(m_view)); > > + > > + ASSERT(ee); > > no newline before ASSERT, it belong to ee
ok
Yael
Comment 30
2012-11-27 15:23:31 PST
Created
attachment 176347
[details]
Patch Address part of
comment #28
.
WebKit Review Bot
Comment 31
2012-11-27 15:57:43 PST
Comment on
attachment 176347
[details]
Patch Clearing flags on attachment: 176347 Committed
r135935
: <
http://trac.webkit.org/changeset/135935
>
WebKit Review Bot
Comment 32
2012-11-27 15:57:50 PST
All reviewed patches have been landed. Closing bug.
Chris Dumez
Comment 33
2012-11-27 23:49:45 PST
This patch seems to have caused 6 failures:
http://build.webkit.org/builders/EFL%20Linux%2064-bit%20Debug%20WK2/builds/6416
We have blurry text on scaled layers and a few other issues.
Jussi Kukkonen (jku)
Comment 34
2012-11-28 03:59:12 PST
(In reply to
comment #33
)
> This patch seems to have caused 6 failures: >
http://build.webkit.org/builders/EFL%20Linux%2064-bit%20Debug%20WK2/builds/6416
> > We have blurry text on scaled layers and a few other issues.
There are over 100 tests that have become flaky ([ IMAGE PASS ]) because of this
Jussi Kukkonen (jku)
Comment 35
2012-11-28 04:23:59 PST
Not sure how it is related but fast/dom/shadow/input-with-validation.html seems to have started crashing reliably after this commit on WK2 debug
Yael
Comment 36
2012-11-28 05:23:25 PST
(In reply to
comment #35
)
> Not sure how it is related but fast/dom/shadow/input-with-validation.html seems to have started crashing reliably after this commit on WK2 debug
We have these bugs to track the crash:
https://bugs.webkit.org/show_bug.cgi?id=98026
https://bugs.webkit.org/show_bug.cgi?id=99886
Dominik Röttsches (drott)
Comment 37
2012-11-29 01:50:10 PST
Causes masive reftests flakiness, this needs to be solved urgently.
Yael
Comment 38
2012-11-29 06:03:16 PST
(In reply to
comment #37
)
> Causes masive reftests flakiness, this needs to be solved urgently.
Unfortunately, many of the failures are real failures. I was looking today e.g. the css/sticky results and the actual results match what I see in MiniBrowser. Which means we have bugs that need to be fixed.
Yael
Comment 39
2012-11-29 07:27:47 PST
(In reply to
comment #37
)
> Causes masive reftests flakiness, this needs to be solved urgently.
Thanks to kbalazs, who helped me analyze some of the flaky tests. It seems that we have a bug in the way we draw the scrollbars. If you look at the expected results images, they are completely wrong in many tests. I will file a separate bug about that.
Yael
Comment 40
2012-11-29 07:33:53 PST
(In reply to
comment #39
)
> (In reply to
comment #37
) > > Causes masive reftests flakiness, this needs to be solved urgently. > > Thanks to kbalazs, who helped me analyze some of the flaky tests. It seems that we have a bug in the way we draw the scrollbars. If you look at the expected results images, they are completely wrong in many tests. I will file a separate bug about that.
https://bugs.webkit.org/show_bug.cgi?id=103641
Jussi Kukkonen (jku)
Comment 41
2012-11-30 06:13:41 PST
(In reply to
comment #38
)
> (In reply to
comment #37
) > > Causes masive reftests flakiness, this needs to be solved urgently. > > Unfortunately, many of the failures are real failures. > I was looking today e.g. the css/sticky results and the actual results match what I see in MiniBrowser. Which means we have bugs that need to be fixed.
Many tests don't seem that way. As an example (and this one seems common) fast/forms/input-first-letter-edit.html is just empty (totally white) when it fails. If you look at the test, it is highly unlikely to be a real failure: it's basically just <body> <input type="text" id="target" /> </body> with a bit of script that 'types' text into the input. I don't think that really renders as an empty page.
Michael Catanzaro
Comment 42
2017-03-11 10:46:05 PST
Closing this bug because the EFL port has been removed from trunk. If you feel this bug applies to a different upstream WebKit port and was closed in error, please either update the title and reopen the bug, or leave a comment to request this.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug