Re: [webkit-dev] Cross-platform fonts for Layout Tests
Unfortunately, even for SVG or images, different drawing implementations will lead to different pixel results. Like this Skia bug, http://code.google.com/p/skia/issues/detail?id=179 , which caused most pages using SkFixed calculation, e.g. round-corner, gradient, svg, etc., produce different rendered image on different platforms. So I think there are many subtle, if possible, issues to make pixels consistent across all platforms. On Tue, Jun 7, 2011 at 1:23 PM, Eric Seidel e...@webkit.org wrote: So are we saying it's impossible to have matching results across all platforms if a test involves any text (in any font)? I know it's certainly possible to have pixel-results for tests which do not involve text match across all platforms (like SVG or images or css styling, etc.) Or is all this just theory? -eric On Mon, Jun 6, 2011 at 8:09 PM, Hao Zheng zheng...@chromium.org wrote: Yes, actually in Skia, Chromium/Linux uses a noop gamma implementation in SkFontHost_gamma_none.cpp; however, if you use a substantial implementation in SkFontHost_gamma.cpp, there will be much image mismatch on Chromium/Linux for every font including Ahem. The slight differences are on font fringe. And I think if other WebKit is not using Skia at all, there will surely be even more differences. As the gamma implementation on Clank is desired, we should not fix this bug. Just file the bug to record this difference. On Fri, Jun 3, 2011 at 2:30 AM, Tony Chang t...@chromium.org wrote: Perhaps, but in practice, it's not enough. Here's an ahem pixel test that is slightly different on Mac and Chromium Linux: http://trac.webkit.org/browser/trunk/LayoutTests/platform/mac/fast/block/basic/010-expected.png http://trac.webkit.org/browser/trunk/LayoutTests/platform/chromium-linux/fast/block/basic/010-expected.png Also, I think it would be hard to tell by examining the HTML if a test uses another font. For example, the line height of an empty block might depend on the default font that isn't specified (does pre/pre render the same height on all platforms?). On Thu, Jun 2, 2011 at 10:44 AM, Adam Barth aba...@webkit.org wrote: I thought the whole point of Ahem was to avoid those problems. Adam On Thu, Jun 2, 2011 at 1:29 AM, Hao Zheng zheng...@chromium.org wrote: Actually, even the same Ahem font will be rendered differently on different platform, depending on the font drawing library, the anti-aliasing algorithm, subpixel, tiny float-point calculation diff on different arch. On Thu, Jun 2, 2011 at 3:30 AM, Eric Seidel e...@webkit.org wrote: I know that Ahem is safe to use across multiple platforms (the font metrics will be the same). Do we know if there are any other fonts for which this is true? I'd like to make the style-bot yell at people when they use pixel tests with non-safe fonts. Right now that list would only include ahem. -eric ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] CSS optional property names
It looks like only the .idl files get pre-processed. The .in files have
comments that start with #, meaning that they cannot be pre-processed by
a C++ preprocessor. We need to change the comment styles for .in files to
//.
I've added https://bugs.webkit.org/show_bug.cgi?id=62114 to track the
changes.
Thanks,
Alex
On 6/6/11 8:35 PM, Eric Seidel e...@webkit.org wrote:
I think our .in files get pre-processed. So you can use normal c++
preprocessor definitions.
#if defined(ENABLE_CSS_REGIONS) ENABLE_CSS_REGIONS
But I could be remembering wrong...
On Mon, Jun 6, 2011 at 4:25 AM, Alexandru Chiculita ach...@adobe.com
wrote:
Hi,
Until we have working CSS Regions and CSS Exclusions implementations we
will
guard the code with ENABLE_CSS_REGIONS and ENABLE_CSS_EXCLUSIONS. We
also
need to guard the property names in CSSPropertyNames.in and
CSSValueKeywords.in using those flags.
Has this been discussed before?
Currently it is done by using different files for optional features. For
example SVG has SVGCSSPropertyNames.in and SVGCSSValueKeywords.in that
are
only used for SVG enabled builds.
Using a C preprocessor will require changing the comment style, because
#
is the prefix for directives in C preprocessors. Should I (1) update the
file to // commenting style and use the C preprocessor or (2) just
implement a simple syntax like the following one?
ENABLE_CSS_EXCLUSIONS {
-webkit-wrap-shape
}
For (1) we could also use the #include directive for the current
approach
#if defined ENABLE_SVG ENABLE_SVG
#include SVGCSSPropertyNames.in
#endif
Regards,
Alex
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] Rounding issues on different platforms
Hi, If you maintain some bots, and you saw rounding issues which makes 1px difference on let's say 32 and 64 bit environments for several layout tests, you may interested in this bug: https://bugs.webkit.org/show_bug.cgi?id=62204 I try to explain what exactly happens there, and it could be used as a master bug for tracking these issues. Regards, Zoltan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Cross-platform fonts for Layout Tests
Reftests? -- Dirk On Mon, Jun 6, 2011 at 11:00 PM, Hao Zheng zheng...@chromium.org wrote: Unfortunately, even for SVG or images, different drawing implementations will lead to different pixel results. Like this Skia bug, http://code.google.com/p/skia/issues/detail?id=179 , which caused most pages using SkFixed calculation, e.g. round-corner, gradient, svg, etc., produce different rendered image on different platforms. So I think there are many subtle, if possible, issues to make pixels consistent across all platforms. On Tue, Jun 7, 2011 at 1:23 PM, Eric Seidel e...@webkit.org wrote: So are we saying it's impossible to have matching results across all platforms if a test involves any text (in any font)? I know it's certainly possible to have pixel-results for tests which do not involve text match across all platforms (like SVG or images or css styling, etc.) Or is all this just theory? -eric On Mon, Jun 6, 2011 at 8:09 PM, Hao Zheng zheng...@chromium.org wrote: Yes, actually in Skia, Chromium/Linux uses a noop gamma implementation in SkFontHost_gamma_none.cpp; however, if you use a substantial implementation in SkFontHost_gamma.cpp, there will be much image mismatch on Chromium/Linux for every font including Ahem. The slight differences are on font fringe. And I think if other WebKit is not using Skia at all, there will surely be even more differences. As the gamma implementation on Clank is desired, we should not fix this bug. Just file the bug to record this difference. On Fri, Jun 3, 2011 at 2:30 AM, Tony Chang t...@chromium.org wrote: Perhaps, but in practice, it's not enough. Here's an ahem pixel test that is slightly different on Mac and Chromium Linux: http://trac.webkit.org/browser/trunk/LayoutTests/platform/mac/fast/block/basic/010-expected.png http://trac.webkit.org/browser/trunk/LayoutTests/platform/chromium-linux/fast/block/basic/010-expected.png Also, I think it would be hard to tell by examining the HTML if a test uses another font. For example, the line height of an empty block might depend on the default font that isn't specified (does pre/pre render the same height on all platforms?). On Thu, Jun 2, 2011 at 10:44 AM, Adam Barth aba...@webkit.org wrote: I thought the whole point of Ahem was to avoid those problems. Adam On Thu, Jun 2, 2011 at 1:29 AM, Hao Zheng zheng...@chromium.org wrote: Actually, even the same Ahem font will be rendered differently on different platform, depending on the font drawing library, the anti-aliasing algorithm, subpixel, tiny float-point calculation diff on different arch. On Thu, Jun 2, 2011 at 3:30 AM, Eric Seidel e...@webkit.org wrote: I know that Ahem is safe to use across multiple platforms (the font metrics will be the same). Do we know if there are any other fonts for which this is true? I'd like to make the style-bot yell at people when they use pixel tests with non-safe fonts. Right now that list would only include ahem. -eric ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Rounding issues on different platforms
On Tue, Jun 7, 2011 at 5:34 AM, Zoltan Herczeg zherc...@inf.u-szeged.hu wrote: If you maintain some bots, and you saw rounding issues which makes 1px difference on let's say 32 and 64 bit environments for several layout tests, you may interested in this bug: https://bugs.webkit.org/show_bug.cgi?id=62204 Years ago someone tracked down a layout test failure to optimizations on a particular file in WebKit. http://code.google.com/p/chromium/issues/detail?id=8475 The extra precision is causing a problem in: third_party/WebKit/WebCore/platform/graphics/Color.cpp which can be fixed by compiling with -ffloat-store. However, that was long enough ago the error may no longer exist in that file. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Rounding issues on different platforms
On Tue, Jun 7, 2011 at 9:33 AM, Evan Martin e...@chromium.org wrote: On Tue, Jun 7, 2011 at 5:34 AM, Zoltan Herczeg zherc...@inf.u-szeged.hu wrote: If you maintain some bots, and you saw rounding issues which makes 1px difference on let's say 32 and 64 bit environments for several layout tests, you may interested in this bug: https://bugs.webkit.org/show_bug.cgi?id=62204 Years ago someone tracked down a layout test failure to optimizations on a particular file in WebKit. http://code.google.com/p/chromium/issues/detail?id=8475 The extra precision is causing a problem in: third_party/WebKit/WebCore/platform/graphics/Color.cpp which can be fixed by compiling with -ffloat-store. However, that was long enough ago the error may no longer exist in that file. I believe I may have found at least one source of rounding issues [1], but I simply have not had time to look into it further. [1] https://bugs.webkit.org/show_bug.cgi?id=54474#c18 --Martin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] CSS optional property names
It looks like only the .idl files get pre-processed. The .in files have comments that start with #, meaning that they cannot be pre-processed by a C++ preprocessor. We need to change the comment styles for .in files to //. That's partly right. Some .in files are preprocessed (tag and attribute names (see Sources/WebCore/html/HTMLTagNames.in for example)) but the CSS ones are not. If you are up to fixing that, all the .in files should have the same processing. Thanks, Julien ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Parallel CSS styling
Eric, You're right that in flickr.com main page, Webkit spends very little time in StyleForElement. However, if you visit http://www.flickr.com/photos/tags/sanfrancisco/ , WebKit spends most of its CSS time in StyleForElement. For example, in our test machine (an 8-core Intel Xeon, 2.8GHz) StyleForElement takes 6450ms out of 9748 ms spent on CSS (66%). Our algorithm focuses on that 66%, and makes it scale linearly. The version of Webkit that we tested includes this patch: Bug 49876 - Optimize matching of descendant selectors Other websites that would benefit: amazon (68% in SFE) Google search (57%) Yahoo sports (56%) Apple (58%) Wikipedia article (65%) -Kulanthaivel Do you have statistics on how much total time rendering flickr.com is in CSS/Style code at all? I believe it to be very low. -eric On Mon, Jun 6, 2011 at 1:16 PM, Kulanthaivel Palanichamy kulanthai...@codeaurora.org wrote: Hi All, At Qualcomm Innovation Center we have been working on a parallel algorithm for CSS styling and wanted to see if there is any interest in the community to see it implemented in WebKit. The overall idea is that we replace CSS matching and styling with a parallel implementation assuming there is a barrier before and after the computation. CSS style application will be performed by the main thread, such that we avoid the need to make thread safe data structures accessed in other passes. The algorithm is task-based, so we would need to implement a thread pool and a simple task scheduler (or maybe use an existing one). In particular, our algorithm requires modifying Element::recalcStyle() and some of the methods it invokes. Code that calls Element::recalcStyle() will not have to be changed. By the time Element::recalStyle() returns, all threads involved on the parallel CSS styling have completed their execution. Effectively, there is a barrier when Element::recalcStyle() begins and another before it returns. Our experiments show that our CSS computation for complex websites scales rather well. For example, we observed that, for flickr.com, Webkit spends 75% of its time in CSS doing CSS matching. Thus, our algorithm would give a maximum speedup of 1.6X on 2 cores and 2.3X on 4 cores. Please let us know whether this would be of interest to the community. -Kulanthaivel ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Parallel CSS styling
You should start by filing a bug at bugs.webkit.org on that 66%. There may be other easier optimizations that don't have all the complexity of threading. Simon On Jun 7, 2011, at 11:22 AM, Kulanthaivel Palanichamy wrote: Eric, You're right that in flickr.com main page, Webkit spends very little time in StyleForElement. However, if you visit http://www.flickr.com/photos/tags/sanfrancisco/ , WebKit spends most of its CSS time in StyleForElement. For example, in our test machine (an 8-core Intel Xeon, 2.8GHz) StyleForElement takes 6450ms out of 9748 ms spent on CSS (66%). Our algorithm focuses on that 66%, and makes it scale linearly. The version of Webkit that we tested includes this patch: Bug 49876 - Optimize matching of descendant selectors Other websites that would benefit: • amazon (68% in SFE) • Google search (57%) • Yahoo sports (56%) • Apple (58%) • Wikipedia article (65%) -Kulanthaivel Do you have statistics on how much total time rendering flickr.com is in CSS/Style code at all? I believe it to be very low. -eric On Mon, Jun 6, 2011 at 1:16 PM, Kulanthaivel Palanichamy kulanthai...@codeaurora.org wrote: Hi All, At Qualcomm Innovation Center we have been working on a parallel algorithm for CSS styling and wanted to see if there is any interest in the community to see it implemented in WebKit. The overall idea is that we replace CSS matching and styling with a parallel implementation assuming there is a barrier before and after the computation. CSS style application will be performed by the main thread, such that we avoid the need to make thread safe data structures accessed in other passes. The algorithm is task-based, so we would need to implement a thread pool and a simple task scheduler (or maybe use an existing one). In particular, our algorithm requires modifying Element::recalcStyle() and some of the methods it invokes. Code that calls Element::recalcStyle() will not have to be changed. By the time Element::recalStyle() returns, all threads involved on the parallel CSS styling have completed their execution. Effectively, there is a barrier when Element::recalcStyle() begins and another before it returns. Our experiments show that our CSS computation for complex websites scales rather well. For example, we observed that, for flickr.com, Webkit spends 75% of its time in CSS doing CSS matching. Thus, our algorithm would give a maximum speedup of 1.6X on 2 cores and 2.3X on 4 cores. Please let us know whether this would be of interest to the community. -Kulanthaivel ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Parallel CSS styling
You noted it spends 66% of its CSS time in StyleForElement. What about total page load time? Then again 6450ms spent in CSS sounds like a lot of time regardless. Answering what % of total page load time we're spending in CSS (or StyleForElement) is important. Loading www.flickr.com/photos/tags/sanfrancisco on my 4-core 2.66ghz Mac Pro doesn't take anywhere near 6 (or for that mater 9!) seconds, so I'm confused how you got 9s in CSS code on a (supposedly faster) machine loading that flickr page. I'm building WebCore so I can shark the page now. Thanks again for your investigation efforts. -eric On Tue, Jun 7, 2011 at 11:22 AM, Kulanthaivel Palanichamy kulanthai...@codeaurora.org wrote: Eric, You're right that in flickr.com main page, Webkit spends very little time in StyleForElement. However, if you visit http://www.flickr.com/photos/tags/sanfrancisco/ , WebKit spends most of its CSS time in StyleForElement. For example, in our test machine (an 8-core Intel Xeon, 2.8GHz) StyleForElement takes 6450ms out of 9748 ms spent on CSS (66%). Our algorithm focuses on that 66%, and makes it scale linearly. The version of Webkit that we tested includes this patch: Bug 49876 - Optimize matching of descendant selectors Other websites that would benefit: • amazon (68% in SFE) • Google search (57%) • Yahoo sports (56%) • Apple (58%) • Wikipedia article (65%) -Kulanthaivel Do you have statistics on how much total time rendering flickr.com is in CSS/Style code at all? I believe it to be very low. -eric On Mon, Jun 6, 2011 at 1:16 PM, Kulanthaivel Palanichamy kulanthai...@codeaurora.org wrote: Hi All, At Qualcomm Innovation Center we have been working on a parallel algorithm for CSS styling and wanted to see if there is any interest in the community to see it implemented in WebKit. The overall idea is that we replace CSS matching and styling with a parallel implementation assuming there is a barrier before and after the computation. CSS style application will be performed by the main thread, such that we avoid the need to make thread safe data structures accessed in other passes. The algorithm is task-based, so we would need to implement a thread pool and a simple task scheduler (or maybe use an existing one). In particular, our algorithm requires modifying Element::recalcStyle() and some of the methods it invokes. Code that calls Element::recalcStyle() will not have to be changed. By the time Element::recalStyle() returns, all threads involved on the parallel CSS styling have completed their execution. Effectively, there is a barrier when Element::recalcStyle() begins and another before it returns. Our experiments show that our CSS computation for complex websites scales rather well. For example, we observed that, for flickr.com, Webkit spends 75% of its time in CSS doing CSS matching. Thus, our algorithm would give a maximum speedup of 1.6X on 2 cores and 2.3X on 4 cores. Please let us know whether this would be of interest to the community. -Kulanthaivel ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Parallel CSS styling
I'm not convinced focusing on single-threaded performance is the best approach. Of course, I support any work done to improve it. Antti's work the past few months has been awesome and I expect there is room for other improvements. However, many of the performance improvements we can make to the single-threaded approach are limited in scope or have other trade-offs. For example, if I understand https://bugs.webkit.org/show_bug.cgi?id=49876 correctly, it only applies during parsing and uses 16 more bytes per RuleData. On some sites (e.g. gmail) that have 30k RuleDatas, that's a ~500k memory hit without much performance benefit. It's a great change for some sites, but it's a significant cost for others. On the other hand, http://trac.webkit.org/changeset/78183 is a strict performance win. It operates by simplifying common codepaths as much as possible to reduce branching. I would be excited if we try to simplify the selector matching code and parallelize it, but, I'm with Eric that we should verify that it's actually on the critical path first. Ojan On Tue, Jun 7, 2011 at 12:16 PM, David Hyatt hy...@apple.com wrote: I'll just re-iterate what Simon was saying. We're very interested in improving CSS performance where it's needed, but I would definitely make sure non-parallel optimization options have been exhausted first. I think there are still some very big gains to be made selector matching even before trying to parallelize it. I'm not necessarily opposed to parallelization, but I believe it's pretty difficult to implement correctly. It's pretty easy to produce a naive parallelization that will look like it works but have correctness and stability bugs. I'm not saying that's what you did, but there are some real subtleties with parallelization that you might not be taking into account. dave (hy...@apple.com) On Jun 7, 2011, at 1:54 PM, Eric Seidel wrote: You noted it spends 66% of its CSS time in StyleForElement. What about total page load time? Then again 6450ms spent in CSS sounds like a lot of time regardless. Answering what % of total page load time we're spending in CSS (or StyleForElement) is important. Loading www.flickr.com/photos/tags/sanfrancisco on my 4-core 2.66ghz Mac Pro doesn't take anywhere near 6 (or for that mater 9!) seconds, so I'm confused how you got 9s in CSS code on a (supposedly faster) machine loading that flickr page. I'm building WebCore so I can shark the page now. Thanks again for your investigation efforts. -eric On Tue, Jun 7, 2011 at 11:22 AM, Kulanthaivel Palanichamy kulanthai...@codeaurora.org wrote: Eric, You're right that in flickr.com main page, Webkit spends very little time in StyleForElement. However, if you visit http://www.flickr.com/photos/tags/sanfrancisco/ , WebKit spends most of its CSS time in StyleForElement. For example, in our test machine (an 8-core Intel Xeon, 2.8GHz) StyleForElement takes 6450ms out of 9748 ms spent on CSS (66%). Our algorithm focuses on that 66%, and makes it scale linearly. The version of Webkit that we tested includes this patch: Bug 49876 - Optimize matching of descendant selectors Other websites that would benefit: • amazon (68% in SFE) • Google search (57%) • Yahoo sports (56%) • Apple (58%) • Wikipedia article (65%) -Kulanthaivel Do you have statistics on how much total time rendering flickr.com is in CSS/Style code at all? I believe it to be very low. -eric On Mon, Jun 6, 2011 at 1:16 PM, Kulanthaivel Palanichamy kulanthai...@codeaurora.org wrote: Hi All, At Qualcomm Innovation Center we have been working on a parallel algorithm for CSS styling and wanted to see if there is any interest in the community to see it implemented in WebKit. The overall idea is that we replace CSS matching and styling with a parallel implementation assuming there is a barrier before and after the computation. CSS style application will be performed by the main thread, such that we avoid the need to make thread safe data structures accessed in other passes. The algorithm is task-based, so we would need to implement a thread pool and a simple task scheduler (or maybe use an existing one). In particular, our algorithm requires modifying Element::recalcStyle() and some of the methods it invokes. Code that calls Element::recalcStyle() will not have to be changed. By the time Element::recalStyle() returns, all threads involved on the parallel CSS styling have completed their execution. Effectively, there is a barrier when Element::recalcStyle() begins and another before it returns. Our experiments show that our CSS computation for complex websites scales rather well. For example, we observed that, for flickr.com, Webkit spends 75% of its time in CSS doing CSS matching. Thus, our algorithm would give a maximum speedup of 1.6X on 2 cores
Re: [webkit-dev] Do we have a style preference about const member functions?
On Fri, Jun 3, 2011 at 5:59 PM, Darin Adler da...@apple.com wrote: On Jun 3, 2011, at 5:46 PM, Peter Kasting wrote: From the perspective of Node itself, I'm not sure why this would be a big task. There shouldn't be any const accessors that return non-const pointers. Simply removing the const qualifier on all the above accessors would make things correct, at the expense of possibly making it difficult to use a const Node*. Maybe you can give it a try and report back. I think you’ll find that this is an enormous task. I now have a local patch which mostly fixes const-correctness of the API declared in Node.h, which I've tested only for Chromium Windows (and only WebCore, no other bits). Before I post it for review somewhere I'd like to get feedback on how to proceed. By fixes, I mean that in most cases, I converted any A* Node::getA() const accessor to a pair of accessors, const A* Node::getA() const and A* Node::getA(). This should flush out people who are trying to perform non-const operations on const pointers, without preventing existing valid usage of const pointers, at the cost of adding a lot of extra accessors that aren't necessarily called. It doesn't guarantee transitive const correctness on other classes besides Node which may have similar accessors, but that's something of an advantage here because it allows me to write a patch to improve Node without having to fix all accessors across all WebCore. For all these pairs of accessors I used the trick from Effective C++ of having the non-const version call the const version and then cast away const on the return type, which looks a little unwieldy at first but guarantees the two accessors cannot get out of step in the future. I avoided fixing Node::scriptExecutionContext() because that's overridden in a zillion different places and I didn't want to touch all of those. I did fix a number of other accessors in other files where they were transitively affected by the changes above. Finally, I made a few decisions on functions to simply make non-const. For example, any accessor which called Document::updateLayoutIgnorePendingStylesheets() was made non-const because that really does have a lot of side-effects. According to jamesr this may be a good thing anyway because apparently we've had bugs (including security bugs) in the past where people call some innocuous-looking const accessor like Node::isContentEditable() and then they end up causing unintentional changes. I'm happy to post this for review somewhere. However, I wonder if first I should try to reduce the pairs of accessors I created above back down to one accessor wherever possible, based on which version is actually called. This would reduce API sprawl at the cost of making the actual APIs somewhat arbitrarily const or not. This might also discourage future users of these classes from re-adding the partner of an existing accessor, in the same way that writing classes without any const accessors encourages people not to ever try to use them via const pointers. Assuming we want this, I would also like to know how we'd like to proceed after this patch. I'm happy to clean up major classes like Element and Document one at a time, but it'd be nice if we settled on a consistent style policy and, if we want to fix a lot of the existing code, if more people than me could help (or if we could automate some things with tooling). PK ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Cross-platform fonts for Layout Tests
For example: fast/borders/borderRadiusArcs01.html svg/W3C-SVG-1.1-SE/pservers-pattern-04-f.svg fast/backgrounds/body-generated-image-propagated-to-root.html Hundreds of tests with skia drawing are affected by this bug, e.g. svg-tests, round-corner, gradient, etc. On Tue, Jun 7, 2011 at 11:58 PM, Dirk Pranke dpra...@chromium.org wrote: Reftests? -- Dirk On Mon, Jun 6, 2011 at 11:00 PM, Hao Zheng zheng...@chromium.org wrote: Unfortunately, even for SVG or images, different drawing implementations will lead to different pixel results. Like this Skia bug, http://code.google.com/p/skia/issues/detail?id=179 , which caused most pages using SkFixed calculation, e.g. round-corner, gradient, svg, etc., produce different rendered image on different platforms. So I think there are many subtle, if possible, issues to make pixels consistent across all platforms. On Tue, Jun 7, 2011 at 1:23 PM, Eric Seidel e...@webkit.org wrote: So are we saying it's impossible to have matching results across all platforms if a test involves any text (in any font)? I know it's certainly possible to have pixel-results for tests which do not involve text match across all platforms (like SVG or images or css styling, etc.) Or is all this just theory? -eric On Mon, Jun 6, 2011 at 8:09 PM, Hao Zheng zheng...@chromium.org wrote: Yes, actually in Skia, Chromium/Linux uses a noop gamma implementation in SkFontHost_gamma_none.cpp; however, if you use a substantial implementation in SkFontHost_gamma.cpp, there will be much image mismatch on Chromium/Linux for every font including Ahem. The slight differences are on font fringe. And I think if other WebKit is not using Skia at all, there will surely be even more differences. As the gamma implementation on Clank is desired, we should not fix this bug. Just file the bug to record this difference. On Fri, Jun 3, 2011 at 2:30 AM, Tony Chang t...@chromium.org wrote: Perhaps, but in practice, it's not enough. Here's an ahem pixel test that is slightly different on Mac and Chromium Linux: http://trac.webkit.org/browser/trunk/LayoutTests/platform/mac/fast/block/basic/010-expected.png http://trac.webkit.org/browser/trunk/LayoutTests/platform/chromium-linux/fast/block/basic/010-expected.png Also, I think it would be hard to tell by examining the HTML if a test uses another font. For example, the line height of an empty block might depend on the default font that isn't specified (does pre/pre render the same height on all platforms?). On Thu, Jun 2, 2011 at 10:44 AM, Adam Barth aba...@webkit.org wrote: I thought the whole point of Ahem was to avoid those problems. Adam On Thu, Jun 2, 2011 at 1:29 AM, Hao Zheng zheng...@chromium.org wrote: Actually, even the same Ahem font will be rendered differently on different platform, depending on the font drawing library, the anti-aliasing algorithm, subpixel, tiny float-point calculation diff on different arch. On Thu, Jun 2, 2011 at 3:30 AM, Eric Seidel e...@webkit.org wrote: I know that Ahem is safe to use across multiple platforms (the font metrics will be the same). Do we know if there are any other fonts for which this is true? I'd like to make the style-bot yell at people when they use pixel tests with non-safe fonts. Right now that list would only include ahem. -eric ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Parallel CSS styling
On Wed, Jun 8, 2011 at 7:54 AM, Ojan Vafai o...@chromium.org wrote: However, many of the performance improvements we can make to the single-threaded approach are limited in scope or have other trade-offs. For example, if I understand https://bugs.webkit.org/show_bug.cgi?id=49876 correctly, it only applies during parsing and uses 16 more bytes per RuleData. On some sites (e.g. gmail) that have 30k RuleDatas, that's a ~500k memory hit without much performance benefit. It's a great change for some sites, but it's a significant cost for others. The point of trade-offs is of course very valid. But rummaging around that part of the code myself, that particular linked patch should actually save quite a bit of matching time: it does a quick'n'dirty comparison against a bloom filter (comparing hashes of IDs/classes/tags of selector parts vs. hashes of stuff we encountered in ancestors) to quickly weed out selectors that cannot possibly match - not all of them, but it should be a significant subset of them. As another possible avenue of improvements I wondered why selectors aren't stored in a Trie data structure. In this way we could 1.) quickly discard whole sub-branches of non-matching selectors on bottom-up matching, and 2.) perhaps even keep track of sub-sets of potentially matching selectors during tree-recursion in a top-down fashion. Cheers, - Roland ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Experimental cr-linux-ews now runs tests
Is it woking for now? I posted a patch which must fail on Chromium-linux, but the EWS didn't post any errors. https://bugs.webkit.org/show_bug.cgi?id=62196 On Tue, May 3, 2011 at 10:37, Adam Barth aba...@webkit.org wrote: The result of the experiment is success! These are now running for real. I'm not sure how much capacity we'll need. I've got three instances running now. We'll see if that's enough. If you see any strange behavior, let me know. Thanks! Adam On Fri, Apr 22, 2011 at 9:08 PM, Adam Barth aba...@webkit.org wrote: I just wanted to let you know that I'm experimenting with having Chromium Linux EWS bots run the LayoutTests. There are a bunch of bugs to work out, but it seems to be more or less working with a bunch of local hacks. Please don't be surprised if the bot acts strangely for a while. Thanks! Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev -- TAMURA Kent Software Engineer, Google ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Experimental cr-linux-ews now runs tests
Why must it fail on Chromium Linux? We're not running pixel tests yet, if that's what you're concerned about. Adam On Tue, Jun 7, 2011 at 9:49 PM, TAMURA, Kent tk...@chromium.org wrote: Is it woking for now? I posted a patch which must fail on Chromium-linux, but the EWS didn't post any errors. https://bugs.webkit.org/show_bug.cgi?id=62196 On Tue, May 3, 2011 at 10:37, Adam Barth aba...@webkit.org wrote: The result of the experiment is success! These are now running for real. I'm not sure how much capacity we'll need. I've got three instances running now. We'll see if that's enough. If you see any strange behavior, let me know. Thanks! Adam On Fri, Apr 22, 2011 at 9:08 PM, Adam Barth aba...@webkit.org wrote: I just wanted to let you know that I'm experimenting with having Chromium Linux EWS bots run the LayoutTests. There are a bunch of bugs to work out, but it seems to be more or less working with a bunch of local hacks. Please don't be surprised if the bot acts strangely for a while. Thanks! Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev -- TAMURA Kent Software Engineer, Google ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Experimental cr-linux-ews now runs tests
* My patch updates the render tree dump of thumbslider-no-parent-slider.html significantly. * Chromium-linux has an old result, platform/chromium-win/fast/forms/thumbslider-no-parent-slider-expected.txt. (Chromium-linux searches chromium-win directory for fallback results.) * test_expectations.txt doesn't contain the test. On Wed, Jun 8, 2011 at 14:05, Adam Barth aba...@webkit.org wrote: Why must it fail on Chromium Linux? We're not running pixel tests yet, if that's what you're concerned about. Adam On Tue, Jun 7, 2011 at 9:49 PM, TAMURA, Kent tk...@chromium.org wrote: Is it woking for now? I posted a patch which must fail on Chromium-linux, but the EWS didn't post any errors. https://bugs.webkit.org/show_bug.cgi?id=62196 On Tue, May 3, 2011 at 10:37, Adam Barth aba...@webkit.org wrote: The result of the experiment is success! These are now running for real. I'm not sure how much capacity we'll need. I've got three instances running now. We'll see if that's enough. If you see any strange behavior, let me know. Thanks! Adam On Fri, Apr 22, 2011 at 9:08 PM, Adam Barth aba...@webkit.org wrote: I just wanted to let you know that I'm experimenting with having Chromium Linux EWS bots run the LayoutTests. There are a bunch of bugs to work out, but it seems to be more or less working with a bunch of local hacks. Please don't be surprised if the bot acts strangely for a while. Thanks! Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev -- TAMURA Kent Software Engineer, Google -- TAMURA Kent Software Engineer, Google ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Experimental cr-linux-ews now runs tests
Very interesting. I'll investigate. Adam On Tue, Jun 7, 2011 at 10:21 PM, TAMURA, Kent tk...@chromium.org wrote: * My patch updates the render tree dump of thumbslider-no-parent-slider.html significantly. * Chromium-linux has an old result, platform/chromium-win/fast/forms/thumbslider-no-parent-slider-expected.txt. (Chromium-linux searches chromium-win directory for fallback results.) * test_expectations.txt doesn't contain the test. On Wed, Jun 8, 2011 at 14:05, Adam Barth aba...@webkit.org wrote: Why must it fail on Chromium Linux? We're not running pixel tests yet, if that's what you're concerned about. Adam On Tue, Jun 7, 2011 at 9:49 PM, TAMURA, Kent tk...@chromium.org wrote: Is it woking for now? I posted a patch which must fail on Chromium-linux, but the EWS didn't post any errors. https://bugs.webkit.org/show_bug.cgi?id=62196 On Tue, May 3, 2011 at 10:37, Adam Barth aba...@webkit.org wrote: The result of the experiment is success! These are now running for real. I'm not sure how much capacity we'll need. I've got three instances running now. We'll see if that's enough. If you see any strange behavior, let me know. Thanks! Adam On Fri, Apr 22, 2011 at 9:08 PM, Adam Barth aba...@webkit.org wrote: I just wanted to let you know that I'm experimenting with having Chromium Linux EWS bots run the LayoutTests. There are a bunch of bugs to work out, but it seems to be more or less working with a bunch of local hacks. Please don't be surprised if the bot acts strangely for a while. Thanks! Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev -- TAMURA Kent Software Engineer, Google -- TAMURA Kent Software Engineer, Google ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
