cc: Combine analysis and raster

cc: Combine analysis and raster

This changes the analysis of PicturePiles from optimistic (hoping to
save raster work by checking if a rastered bitmap would be a solid
color) to pessimistic (assuming most bitmaps are not solid colors and
just checking afterwards).  It does this by rastering to both an
AnalysisCanvas and a normal SkCanvas via SkNWayCanvas and then
checking the results afterwards.

R=reveman@chromium.org, senorblanco@chromium.org
BUG=310796
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=234641

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=243268

[enne (OOO), 2013-11-07 00:58:18]

In my measurements the analysis step on an N4 across a good number of pages
seemed to be about 1/4 to 1/2 of a ms, which can add up for many tiles.  On
simple pages where rastering takes 0.5ms, analysis was 50% of the raster cost.

Analyzing a pile as it is being rastered did not appear to add any overhead
significant enough to be noticable above noise.  I instrumented the raster
worker pool to do three rasters: one raster to warm up the image cache, one
raster with analysis, and one raster without analysis.  I then ran that build
over 5-6 pages and compared the analysis + raster as separate steps vs the
analyis + raster in the same step.

This seemed worth implementing although it was not a huge win overall.  I
suspect the recent changes to PicturePile which reduced the number of SkPictures
needing to be analyzed per raster call has made the analysis costs a lot less
recently.

https://codereview.chromium.org/63443003/diff/1/cc/resources/raster_worker_po...
File cc/resources/raster_worker_pool.cc (left):

https://codereview.chromium.org/63443003/diff/1/cc/resources/raster_worker_po...
cc/resources/raster_worker_pool.cc:36: const bool kUseColorEstimator = true;
This no longer seemed useful, so I removed it.

https://codereview.chromium.org/63443003/diff/1/skia/ext/analysis_canvas.h
File skia/ext/analysis_canvas.h (right):

https://codereview.chromium.org/63443003/diff/1/skia/ext/analysis_canvas.h#ne...
skia/ext/analysis_canvas.h:18: // Gathers statistics about a region of an
SkPicture as the picture is played
cc is the only user of AnalysisCanvas, so I changed it in place.

https://codereview.chromium.org/63443003/diff/1/skia/ext/analysis_canvas.h#ne...
skia/ext/analysis_canvas.h:60: // |analysis_rect| is in SkCanvas space.
Previously the analyze step would size the SkBitmap to the analyze rect and
implicitly the AnalysisCanvas and AnalysisDevice would use that size to figure
out if the entire area was covered.

This just makes that part explicit so that the SkBitmap can be larger than the
analysis rect (which it is [and needs to be] for edge tiles).

(Also, should "SkCanvas space" be called device space?)

[vangelis, 2013-11-07 01:35:03]

https://codereview.chromium.org/63443003/diff/1/skia/ext/analysis_canvas.cc
File skia/ext/analysis_canvas.cc (right):

https://codereview.chromium.org/63443003/diff/1/skia/ext/analysis_canvas.cc#n...
skia/ext/analysis_canvas.cc:126: INHERITED::drawPaint(draw, paint);
Do we see this method getting called for common web content?  If so, it may be
worth including it in the analysis as it could clear out a large chunk of
pixels.

[Tom Hudson, 2013-11-07 13:47:11]

If it's not giving us any speedup, why leave the code there at all?

[reveman, 2013-11-07 15:13:41]

On 2013/11/07 13:47:11, Tom Hudson wrote:
> If it's not giving us any speedup, why leave the code there at all?

Reduced gpu memory usage, more efficient drawing of the tile and we can skip the
upload when using PixelBufferRWP.

[Tom Hudson, 2013-11-07 15:28:54]

On 2013/11/07 15:13:41, David Reveman wrote:
> On 2013/11/07 13:47:11, Tom Hudson wrote:
> > If it's not giving us any speedup, why leave the code there at all?
> 
> Reduced gpu memory usage, more efficient drawing of the tile and we can skip
the
> upload when using PixelBufferRWP.

I can see how we get reduced GPU memory usage and skip uploads, but more
efficient drawing of the tile? Aren't we drawing the tile first, and then doing
the analysis?

[reveman, 2013-11-07 15:36:30]

https://codereview.chromium.org/63443003/diff/1/cc/resources/picture_pile_imp...
File cc/resources/picture_pile_impl.cc (left):

https://codereview.chromium.org/63443003/diff/1/cc/resources/picture_pile_imp...
cc/resources/picture_pile_impl.cc:204: SkDrawPictureCallback* callback,
|callback| doesn't seem to be used anymore. Should we remove it?

https://codereview.chromium.org/63443003/diff/1/cc/resources/raster_worker_po...
File cc/resources/raster_worker_pool.cc (right):

https://codereview.chromium.org/63443003/diff/1/cc/resources/raster_worker_po...
cc/resources/raster_worker_pool.cc:183: return RunRasterOnThread(thread_index,
buffer, size, stride);
A separate RunRasterOnThread function is not necessary anymore. Please move all
the above code into RunOnWorkerThread instead.

https://codereview.chromium.org/63443003/diff/1/skia/ext/analysis_canvas.h
File skia/ext/analysis_canvas.h (right):

https://codereview.chromium.org/63443003/diff/1/skia/ext/analysis_canvas.h#ne...
skia/ext/analysis_canvas.h:22: AnalysisCanvas(AnalysisDevice* device);
explicit?

[reveman, 2013-11-07 15:40:31]

On 2013/11/07 15:28:54, Tom Hudson wrote:
> On 2013/11/07 15:13:41, David Reveman wrote:
> > On 2013/11/07 13:47:11, Tom Hudson wrote:
> > > If it's not giving us any speedup, why leave the code there at all?
> > 
> > Reduced gpu memory usage, more efficient drawing of the tile and we can skip
> the
> > upload when using PixelBufferRWP.
> 
> I can see how we get reduced GPU memory usage and skip uploads, but more
> efficient drawing of the tile? Aren't we drawing the tile first, and then
doing
> the analysis?

Drawing of the tile onto screen using GL. We'd use a solid color shader rather
than having to sample from a texture.

[vmpstr, 2013-11-07 17:36:46]

Did you by any chance try using SkNWayCanvas to see what is the time difference?
I'm wondering if the cost of analysis is dominated by actual AnalysisCanvas
operations or walking the SkPicture. Also, it would allow us to keep
AnalysisCanvas as just analysis, although I don't really have any good use cases
for it aside from what it currently does.

[Stephen White, 2013-11-07 18:14:21]

LGTM for skia/, but will leave the rest for CC experts.

https://codereview.chromium.org/63443003/diff/1/cc/test/fake_picture_pile_imp...
File cc/test/fake_picture_pile_impl.cc (right):

https://codereview.chromium.org/63443003/diff/1/cc/test/fake_picture_pile_imp...
cc/test/fake_picture_pile_impl.cc:104:
empty_bitmap.setConfig(SkBitmap::kNo_Config,
Nit: I find it a bit weird that this API uses kNo_Config to indicate a new
bitmap, but I see that that usage predates this CL. I wonder if there might be a
cleaner way, perhaps using getTexture() for GPU, or getPixels() for raster.

https://codereview.chromium.org/63443003/diff/1/skia/ext/analysis_canvas.cc
File skia/ext/analysis_canvas.cc (right):

https://codereview.chromium.org/63443003/diff/1/skia/ext/analysis_canvas.cc#n...
skia/ext/analysis_canvas.cc:408: // - Transfer mode is clear (0 color, 0 alpha)
Nit: also not new to this patch, but is it possible that an SkShader in the
paint could override the transfer mode and end up painting non-transparent here?
Don't know, but perhaps reed@ does. If so, I wonder if we should have some
helper function on SkPaint to query when a paint will definitely paint
transparent black (if that's what this test is trying to do).

https://codereview.chromium.org/63443003/diff/1/skia/ext/analysis_canvas.h
File skia/ext/analysis_canvas.h (right):

https://codereview.chromium.org/63443003/diff/1/skia/ext/analysis_canvas.h#ne...
skia/ext/analysis_canvas.h:60: // |analysis_rect| is in SkCanvas space.
On 2013/11/07 00:58:18, enne wrote:
> Previously the analyze step would size the SkBitmap to the analyze rect and
> implicitly the AnalysisCanvas and AnalysisDevice would use that size to figure
> out if the entire area was covered.
> 
> This just makes that part explicit so that the SkBitmap can be larger than the
> analysis rect (which it is [and needs to be] for edge tiles).
> 
> (Also, should "SkCanvas space" be called device space?)

From the Skia side, SkCanvas space is not really device space, since coords sent
to SkCanvas are pre-CTM. (If I understand your question correctly.)

[reed1, 2013-11-07 18:48:05]

somewhat orthogonal to this CL...

Can all of the analysis be done just at the canvas level? Curious why there is
both a special canvas and special device, and that the caller has to be aware of
this dualism.

[enne (OOO), 2013-11-07 22:13:49]

On 2013/11/07 13:47:11, Tom Hudson wrote:
> If it's not giving us any speedup, why leave the code there at all?

It does give us a speed-up.  This change branch predicts that we'll have so few
solid color tiles that it's worth wasting the time rasterizing them (which will
probably be cheap anyway) rather than wasting time up front in the common case
analyzing mostly not-solid tiles.

On 2013/11/07 17:36:46, vmpstr wrote:
> Did you by any chance try using SkNWayCanvas to see what is the time
difference?
> I'm wondering if the cost of analysis is dominated by actual AnalysisCanvas
> operations or walking the SkPicture. Also, it would allow us to keep
> AnalysisCanvas as just analysis, although I don't really have any good use
cases
> for it aside from what it currently does.

I didn't try it out.  My thought is that if we end up having a user of
AnalysisCanvas that wants it to be a no-op, then we can refactor it then and see
if we can reuse it with an n-way canvas.

On 2013/11/07 18:48:05, reed1 wrote:
> Can all of the analysis be done just at the canvas level? Curious why there is
> both a special canvas and special device, and that the caller has to be aware
of
> this dualism.

I don't know.  If somebody with more Skia knowledge wants to simplify this, then
have at it.

https://codereview.chromium.org/63443003/diff/1/cc/resources/picture_pile_imp...
File cc/resources/picture_pile_impl.cc (left):

https://codereview.chromium.org/63443003/diff/1/cc/resources/picture_pile_imp...
cc/resources/picture_pile_impl.cc:204: SkDrawPictureCallback* callback,
On 2013/11/07 15:36:31, David Reveman wrote:
> |callback| doesn't seem to be used anymore. Should we remove it?

Done.

https://codereview.chromium.org/63443003/diff/1/cc/resources/raster_worker_po...
File cc/resources/raster_worker_pool.cc (right):

https://codereview.chromium.org/63443003/diff/1/cc/resources/raster_worker_po...
cc/resources/raster_worker_pool.cc:183: return RunRasterOnThread(thread_index,
buffer, size, stride);
On 2013/11/07 15:36:31, David Reveman wrote:
> A separate RunRasterOnThread function is not necessary anymore. Please move
all
> the above code into RunOnWorkerThread instead.

Done.

https://codereview.chromium.org/63443003/diff/1/skia/ext/analysis_canvas.cc
File skia/ext/analysis_canvas.cc (right):

https://codereview.chromium.org/63443003/diff/1/skia/ext/analysis_canvas.cc#n...
skia/ext/analysis_canvas.cc:126: INHERITED::drawPaint(draw, paint);
On 2013/11/07 01:35:03, vangelis wrote:
> Do we see this method getting called for common web content?  If so, it may be
> worth including it in the analysis as it could clear out a large chunk of
> pixels.

No.  This is not called from Blink.

https://codereview.chromium.org/63443003/diff/1/skia/ext/analysis_canvas.h
File skia/ext/analysis_canvas.h (right):

https://codereview.chromium.org/63443003/diff/1/skia/ext/analysis_canvas.h#ne...
skia/ext/analysis_canvas.h:22: AnalysisCanvas(AnalysisDevice* device);
On 2013/11/07 15:36:31, David Reveman wrote:
> explicit?

Done.

https://codereview.chromium.org/63443003/diff/1/skia/ext/analysis_canvas.h#ne...
skia/ext/analysis_canvas.h:60: // |analysis_rect| is in SkCanvas space.
On 2013/11/07 18:14:22, Stephen White wrote:
> On 2013/11/07 00:58:18, enne wrote:
> > Previously the analyze step would size the SkBitmap to the analyze rect and
> > implicitly the AnalysisCanvas and AnalysisDevice would use that size to
figure
> > out if the entire area was covered.
> > 
> > This just makes that part explicit so that the SkBitmap can be larger than
the
> > analysis rect (which it is [and needs to be] for edge tiles).
> > 
> > (Also, should "SkCanvas space" be called device space?)
> 
> From the Skia side, SkCanvas space is not really device space, since coords
sent
> to SkCanvas are pre-CTM. (If I understand your question correctly.)

Ok, so this should say device space to be what I intend it to be.

[reveman, 2013-11-07 22:20:16]

cc/ lgtm with nit

https://codereview.chromium.org/63443003/diff/170001/cc/resources/raster_work...
File cc/resources/raster_worker_pool.cc (right):

https://codereview.chromium.org/63443003/diff/170001/cc/resources/raster_work...
cc/resources/raster_worker_pool.cc:78: "cc",
"RasterWorkerPoolTaskImpl::RunRasterOnThread",
nit: RunOnWorkerThread

https://codereview.chromium.org/63443003/diff/170001/tools/vim/ninja-build.vim
File tools/vim/ninja-build.vim (right):

https://codereview.chromium.org/63443003/diff/170001/tools/vim/ninja-build.vi...
tools/vim/ninja-build.vim:112: if (l:targets !~ "\\i")
is this intentional?

[enne (OOO), 2013-11-07 22:32:08]

https://codereview.chromium.org/63443003/diff/170001/cc/resources/raster_work...
File cc/resources/raster_worker_pool.cc (right):

https://codereview.chromium.org/63443003/diff/170001/cc/resources/raster_work...
cc/resources/raster_worker_pool.cc:78: "cc",
"RasterWorkerPoolTaskImpl::RunRasterOnThread",
On 2013/11/07 22:20:17, David Reveman wrote:
> nit: RunOnWorkerThread

TraceViewer uses this string directly.  If I'm going to change it, I think I'd
rather do it separately and maybe move all of these strings to their own file. 
*punt*, sorry.

https://codereview.chromium.org/63443003/diff/170001/tools/vim/ninja-build.vim
File tools/vim/ninja-build.vim (right):

https://codereview.chromium.org/63443003/diff/170001/tools/vim/ninja-build.vi...
tools/vim/ninja-build.vim:112: if (l:targets !~ "\\i")
On 2013/11/07 22:20:17, David Reveman wrote:
> is this intentional?

OOPS

[enne (OOO), 2013-11-07 22:36:09]

https://codereview.chromium.org/63443003/diff/1/cc/test/fake_picture_pile_imp...
File cc/test/fake_picture_pile_impl.cc (right):

https://codereview.chromium.org/63443003/diff/1/cc/test/fake_picture_pile_imp...
cc/test/fake_picture_pile_impl.cc:104:
empty_bitmap.setConfig(SkBitmap::kNo_Config,
On 2013/11/07 18:14:22, Stephen White wrote:
> Nit: I find it a bit weird that this API uses kNo_Config to indicate a new
> bitmap, but I see that that usage predates this CL. I wonder if there might be
a
> cleaner way, perhaps using getTexture() for GPU, or getPixels() for raster.

If there's a better way to fix this, please let me know or uploa da patch.  I am
doing this because other code did this and not because I know what I should be
doing with Skia.

https://codereview.chromium.org/63443003/diff/1/skia/ext/analysis_canvas.cc
File skia/ext/analysis_canvas.cc (right):

https://codereview.chromium.org/63443003/diff/1/skia/ext/analysis_canvas.cc#n...
skia/ext/analysis_canvas.cc:408: // - Transfer mode is clear (0 color, 0 alpha)
On 2013/11/07 18:14:22, Stephen White wrote:
> Nit: also not new to this patch, but is it possible that an SkShader in the
> paint could override the transfer mode and end up painting non-transparent
here?
> Don't know, but perhaps reed@ does. If so, I wonder if we should have some
> helper function on SkPaint to query when a paint will definitely paint
> transparent black (if that's what this test is trying to do).

Such a helper function from the Skia side would be really helpful.

[commit-bot: I haz the power, 2013-11-07 22:42:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/enne@chromium.org/63443003/240001

[commit-bot: I haz the power, 2013-11-07 23:22:47]

Sorry for I got bad news for ya.
Compile failed with a clobber build on android_clang_dbg.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_cl...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-11-08 00:25:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/enne@chromium.org/63443003/490001

[commit-bot: I haz the power, 2013-11-08 01:21:10]

Retried try job too often on linux_rel for step(s) base_unittests,
browser_tests, cacheinvalidation_unittests, cc_unittests, check_deps,
chromedriver2_unittests, components_unittests, content_browsertests,
content_unittests, crypto_unittests, gpu_unittests, interactive_ui_tests,
ipc_tests, jingle_unittests, media_unittests, nacl_integration, net_unittests,
ppapi_unittests, printing_unittests, remoting_unittests,
sandbox_linux_unittests, sql_unittests, sync_integration_tests, sync_unit_tests,
unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[enne (OOO), 2013-11-08 23:15:55]

Argh.  I had missed something fundamental here.  AnalysisDevice wants to pretend
like it's the only device around and intercept all calls.  If I make saveLayer
truly saveLayer, then that creates a new device.  I could rewrite AnalysisCanvas
to deal with multiple AnalysisDevices, all analyzing their own bitmap, but that
seems like a lot of extra complication and needless change.

I've instead rewritten this to just use an SkNWayCanvas as vmpstr suggested and
have reverted the pass-through changes to AnalysisCanvas and AnalysisDevice,
leaving them noops as they were before.  This should now pass all tests.

[vmpstr, 2013-11-08 23:21:26]

On 2013/11/08 23:15:55, enne wrote:
> Argh.  I had missed something fundamental here.  AnalysisDevice wants to
pretend
> like it's the only device around and intercept all calls.  If I make saveLayer
> truly saveLayer, then that creates a new device.  I could rewrite
AnalysisCanvas
> to deal with multiple AnalysisDevices, all analyzing their own bitmap, but
that
> seems like a lot of extra complication and needless change.
> 
> I've instead rewritten this to just use an SkNWayCanvas as vmpstr suggested
and
> have reverted the pass-through changes to AnalysisCanvas and AnalysisDevice,
> leaving them noops as they were before.  This should now pass all tests.

Do you know how it performs as compared to the other approaches? (modulo noise)

[vangelis, 2013-11-08 23:44:07]

https://codereview.chromium.org/63443003/diff/870001/cc/resources/picture_pil...
File cc/resources/picture_pile_impl.cc (right):

https://codereview.chromium.org/63443003/diff/870001/cc/resources/picture_pil...
cc/resources/picture_pile_impl.cc:129: RasterCommon(canvas,
Since we can no longer make use of the early-aborts in playback for the analysis
canvas, should we early-out more aggressively from some of the more complex
AnalysisCanvas methods (e.g. saveLayer, clipPath, drawRect) if we've already
determined that:

1. is_transparent == false;
2. has_text = true;
3. is_solid_color = false;

which are probably going to be quite common ?

[enne (OOO), 2013-11-08 23:47:59]

https://codereview.chromium.org/63443003/diff/870001/cc/resources/picture_pil...
File cc/resources/picture_pile_impl.cc (right):

https://codereview.chromium.org/63443003/diff/870001/cc/resources/picture_pil...
cc/resources/picture_pile_impl.cc:129: RasterCommon(canvas,
On 2013/11/08 23:44:08, vangelis wrote:
> Since we can no longer make use of the early-aborts in playback for the
analysis
> canvas, should we early-out more aggressively from some of the more complex
> AnalysisCanvas methods (e.g. saveLayer, clipPath, drawRect) if we've already
> determined that:
> 
> 1. is_transparent == false;
> 2. has_text = true;
> 3. is_solid_color = false;
> 
> which are probably going to be quite common ? 

I'm not sure it's worth the complication.  I didn't really see non-aborting
analysis add much overhead.  If it shows up, we can early out.

[vangelis, 2013-11-08 23:53:13]

On 2013/11/08 23:47:59, enne wrote:
>
https://codereview.chromium.org/63443003/diff/870001/cc/resources/picture_pil...
> File cc/resources/picture_pile_impl.cc (right):
> 
>
https://codereview.chromium.org/63443003/diff/870001/cc/resources/picture_pil...
> cc/resources/picture_pile_impl.cc:129: RasterCommon(canvas,
> On 2013/11/08 23:44:08, vangelis wrote:
> > Since we can no longer make use of the early-aborts in playback for the
> analysis
> > canvas, should we early-out more aggressively from some of the more complex
> > AnalysisCanvas methods (e.g. saveLayer, clipPath, drawRect) if we've already
> > determined that:
> > 
> > 1. is_transparent == false;
> > 2. has_text = true;
> > 3. is_solid_color = false;
> > 
> > which are probably going to be quite common ? 
> 
> I'm not sure it's worth the complication.  I didn't really see non-aborting
> analysis add much overhead.  If it shows up, we can early out.

Fair enough although I'm not sure how we would see the overhead (it will be
spread out). Do you have sense of how much more expensive the RasterCommon gets
when running through this nway canvas as opposed to running with a single
canvas?

[enne (OOO), 2013-11-09 00:57:02]

On 2013/11/08 23:53:13, vangelis wrote:
> Fair enough although I'm not sure how we would see the overhead (it will be
> spread out). Do you have sense of how much more expensive the RasterCommon
gets
> when running through this nway canvas as opposed to running with a single
> canvas?

In a few traces, it doesn't look like any more overhead.  Or, if there is
overhead, it's in the noise and seems about the same as the previous approach.

I did go ahead and add an early out to draw rect, since that's a map.  The other
two cases you point out don't seem that much more complicated.  clipPath just
becomes clipRect and saveLayer doesn't seem safe to skip but also doesn't do
much work.

[enne (OOO), 2013-11-11 19:27:56]

It doesn't sound like anybody has objections to this approach other than
performance, so I am going to go ahead and land this.

[commit-bot: I haz the power, 2013-11-11 19:29:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/enne@chromium.org/63443003/1050001

[commit-bot: I haz the power, 2013-11-11 20:20:04]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-11-12 00:35:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/enne@chromium.org/63443003/1050001

[commit-bot: I haz the power, 2013-11-12 01:40:12]

Retried try job too often on mac_rel for step(s) unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2013-11-12 18:43:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/enne@chromium.org/63443003/1430001

[commit-bot: I haz the power, 2013-11-12 21:35:33]

Change committed as 234641

[enne (OOO), 2013-12-17 01:10:42]

Reopening for relanding.  The original patch got reverted due to the
SkNWayCanvas kerfuffle.  Then I needed to make the old raster and record
benchmark also do analysis as well.  That's been done too.  Now we're back full
circle to landing the original patch.

I'll take a look at the perf bots and make sure they've had enough time to chew
on https://codereview.chromium.org/74713006/ before landing this.

PTAL if you'd like, but there shouldn't be any substantive changes here.

[commit-bot: I haz the power, 2014-01-07 00:02:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/enne@chromium.org/63443003/1730001

[commit-bot: I haz the power, 2014-01-07 11:57:24]

Change committed as 243268

[enne (OOO), 2014-01-07 19:54:14]

A revert of this CL has been created in
https://codereview.chromium.org/126343003/ by enne@chromium.org.

The reason for reverting is: Caused a 30% rasterization regression.