Commit 9cde44e2 authored by aiolos's avatar aiolos Committed by Commit bot

Revert of Simplify IOSurface CoreAnimation code: Part 2 (patchset #13...

Revert of Simplify IOSurface CoreAnimation code: Part 2 (patchset #13 id:240001 of https://codereview.chromium.org/490393002/)

Reason for revert:
crbug.com/410360	(Browser crashing on IOSurfaceLayer release in tab_switching.typical_25)

Original issue's description:
> Simplify IOSurface CoreAnimation code: Part 2
>
> It isn't necessary to mess around with sharing CGLContextObjs anymore,
> they were necessary only to share data between the frame subscriber
> and the RenderWidgetHostViewMac. Just give each IOSurfaceLayer its
> own CGLContextObj, and draw using immediate mode (using shaders
> resulted in an increase in startup time).
>
> BUG=314190
> TBR=pfeldman@chromium.org
> R=kbr@chromium.org
>
> Committed: https://chromium.googlesource.com/chromium/src/+/a83898264f64ad83bb98139e5e0dcb2c77db1997

TBR=kbr@chromium.org,ccameron@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=314190

Review URL: https://codereview.chromium.org/528403002

Cr-Commit-Position: refs/heads/master@{#293174}
parent 2955e423
......@@ -208,6 +208,7 @@ void BrowserCompositorViewMacInternal::GotAcceleratedIOSurfaceFrame(
// layer.
bool needs_new_layer =
!io_surface_layer_ ||
[io_surface_layer_ hasBeenPoisoned] ||
[io_surface_layer_ scaleFactor] != scale_factor;
if (needs_new_layer) {
io_surface_layer_.reset(
......@@ -220,12 +221,28 @@ void BrowserCompositorViewMacInternal::GotAcceleratedIOSurfaceFrame(
}
// Open the provided IOSurface.
[io_surface_layer_ gotFrameWithIOSurface:io_surface_id
withPixelSize:pixel_size
withScaleFactor:scale_factor];
if (io_surface_layer_) {
bool result = [io_surface_layer_ gotFrameWithIOSurface:io_surface_id
withPixelSize:pixel_size
withScaleFactor:scale_factor];
if (!result) {
DestroyIOSurfaceLayer(io_surface_layer_);
LOG(ERROR) << "Failed open IOSurface in IOSurfaceLayer";
}
}
// Give a final complaint if anything with the layer's creation went wrong.
// This frame will appear blank, the compositor will try to create another,
// and maybe that will go better.
if (!io_surface_layer_) {
LOG(ERROR) << "IOSurfaceLayer is nil, tab will be blank";
IOSurfaceLayerHitError();
}
// Make the CALayer draw and set its size appropriately.
if (io_surface_layer_) {
[io_surface_layer_ gotNewFrame];
// Set the bounds of the accelerated layer to match the size of the frame.
// If the bounds changed, force the content to be displayed immediately.
CGRect new_layer_bounds = CGRectMake(
......@@ -328,8 +345,12 @@ void BrowserCompositorViewMacInternal::IOSurfaceLayerDidDrawFrame() {
}
void BrowserCompositorViewMacInternal::IOSurfaceLayerHitError() {
// Perform all acks that would have been done if the frame had succeeded, to
// un-block the compositor and renderer.
IOSurfaceLayerDidDrawFrame();
// Poison the context being used and request a mulligan.
DestroyIOSurfaceLayer(io_surface_layer_);
[io_surface_layer_ poisonContextAndSharegroup];
compositor_->ScheduleFullRedraw();
}
......
......@@ -2,11 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef CONTENT_BROWSER_COMPOSITOR_IO_SURFACE_LAYER_MAC_H_
#define CONTENT_BROWSER_COMPOSITOR_IO_SURFACE_LAYER_MAC_H_
#ifndef CONTENT_BROWSER_RENDERER_HOST_COMPOSITING_IOSURFACE_LAYER_MAC_H_
#define CONTENT_BROWSER_RENDERER_HOST_COMPOSITING_IOSURFACE_LAYER_MAC_H_
#import <Cocoa/Cocoa.h>
#include <IOSurface/IOSurfaceAPI.h>
#include "base/mac/scoped_cftyperef.h"
#include "base/memory/ref_counted.h"
......@@ -16,7 +15,8 @@
@class IOSurfaceLayer;
namespace content {
class IOSurfaceLayerHelper;
class CompositingIOSurfaceMac;
class CompositingIOSurfaceContext;
// The interface through which the IOSurfaceLayer calls back into
// the structrue that created it (RenderWidgetHostViewMac or
......@@ -36,13 +36,58 @@ class IOSurfaceLayerClient {
virtual void IOSurfaceLayerHitError() = 0;
};
} // namespace content
// IOSurfaceLayerHelper provides C++ functionality needed for the
// IOSurfaceLayer class, and does most of the heavy lifting for the
// class.
// TODO(ccameron): This class should own IOSurfaceLayer, rather than
// vice versa.
class IOSurfaceLayerHelper {
public:
IOSurfaceLayerHelper(IOSurfaceLayerClient* client,
IOSurfaceLayer* layer);
~IOSurfaceLayerHelper();
// The CoreAnimation layer for drawing accelerated content.
@interface IOSurfaceLayer : CAOpenGLLayer {
@private
content::IOSurfaceLayerClient* client_;
scoped_ptr<content::IOSurfaceLayerHelper> helper_;
// Called when the IOSurfaceLayer gets a new frame.
void GotNewFrame();
// Called whenever -[IOSurfaceLayer setNeedsDisplay] is called.
void SetNeedsDisplay();
// Called whenever -[IOSurfaceLayer canDrawInCGLContext] is called,
// to determine if a new frame should be drawn.
bool CanDraw();
// Called whenever -[IOSurfaceLayer drawInCGLContext] draws a
// frame.
void DidDraw(bool success);
// Immediately re-draw the layer, even if the content has not changed, and
// ensure that the frame be acked.
void SetNeedsDisplayAndDisplayAndAck();
// Immediately draw the layer, only if one is pending, and ensure that the
// frame be acked.
void DisplayIfNeededAndAck();
// Mark a bracket in which new frames are being pumped in a restricted nested
// run loop. During this time frames are acked immediately and draws are
// deferred until the bracket ends.
void BeginPumpingFrames();
void EndPumpingFrames();
private:
// Called whenever the frame provided in GotNewFrame should be acknowledged
// (this may be because it was drawn, or it may be to unblock the
// compositor).
void AckPendingFrame(bool success);
void TimerFired();
// The client that the owning layer was created with.
content::IOSurfaceLayerClient* const client_;
// The layer that owns this helper.
IOSurfaceLayer* const layer_;
// Used to track when canDrawInCGLContext should return YES. This can be
// in response to receiving a new compositor frame, or from any of the events
......@@ -59,30 +104,35 @@ class IOSurfaceLayerClient {
// Set when inside a BeginPumpingFrames/EndPumpingFrames block.
bool is_pumping_frames_;
// The IOSurface being drawn by this layer.
base::ScopedCFTypeRef<IOSurfaceRef> io_surface_;
// The browser places back-pressure on the GPU by not acknowledging swap
// calls until they appear on the screen. This can lead to hangs if the
// view is moved offscreen (among other things). Prevent hangs by always
// acknowledging the frame after timeout of 1/6th of a second has passed.
base::DelayTimer<IOSurfaceLayerHelper> timer_;
};
// The size of the frame in pixels. This will be less or equal than the pixel
// size of |io_surface_|.
gfx::Size frame_pixel_size_;
} // namespace content
// The GL texture that is bound to |io_surface_|. If |io_surface_| changes,
// then this is marked as dirty by setting |io_surface_texture_dirty_|.
GLuint io_surface_texture_;
bool io_surface_texture_dirty_;
// The CoreAnimation layer for drawing accelerated content.
@interface IOSurfaceLayer : CAOpenGLLayer {
@private
scoped_refptr<content::CompositingIOSurfaceMac> iosurface_;
scoped_refptr<content::CompositingIOSurfaceContext> context_;
// The CGL renderer ID, captured at draw time.
GLint cgl_renderer_id_;
scoped_ptr<content::IOSurfaceLayerHelper> helper_;
}
- (id)initWithClient:(content::IOSurfaceLayerClient*)client
withScaleFactor:(float)scale_factor;
// Called when a new frame is received.
- (void)gotFrameWithIOSurface:(IOSurfaceID)io_surface_id
- (bool)gotFrameWithIOSurface:(IOSurfaceID)io_surface_id
withPixelSize:(gfx::Size)pixel_size
withScaleFactor:(float)scale_factor;
// Context poison accessors.
- (void)poisonContextAndSharegroup;
- (bool)hasBeenPoisoned;
- (float)scaleFactor;
// The CGL renderer ID.
......@@ -92,14 +142,20 @@ class IOSurfaceLayerClient {
// must be called before the layer is destroyed.
- (void)resetClient;
// Called when a new frame is received.
- (void)gotNewFrame;
// Force a draw immediately (even if this means re-displaying a previously
// displayed frame).
- (void)setNeedsDisplayAndDisplayAndAck;
// Force a draw immediately, but only if one was requested.
- (void)displayIfNeededAndAck;
// Mark a bracket in which new frames are being pumped in a restricted nested
// run loop.
- (void)beginPumpingFrames;
- (void)endPumpingFrames;
@end
#endif // CONTENT_BROWSER_COMPOSITOR_IO_SURFACE_LAYER_MAC_H_
#endif // CONTENT_BROWSER_RENDERER_HOST_COMPOSITING_IOSURFACE_LAYER_MAC_H_
......@@ -125,9 +125,9 @@ IN_PROC_BROWSER_TEST_F(CaptureScreenshotTest, MAYBE_CaptureScreenshot) {
gfx::PNGCodec::Decode(reinterpret_cast<const unsigned char*>(png.data()),
png.size(), &bitmap);
SkColor color(bitmap.getColor(0, 0));
EXPECT_LE(std::abs(0x12-(int)SkColorGetR(color)), 1);
EXPECT_LE(std::abs(0x34-(int)SkColorGetG(color)), 1);
EXPECT_LE(std::abs(0x56-(int)SkColorGetB(color)), 1);
EXPECT_TRUE(std::abs(0x12-(int)SkColorGetR(color)) <= 1);
EXPECT_TRUE(std::abs(0x34-(int)SkColorGetG(color)) <= 1);
EXPECT_TRUE(std::abs(0x56-(int)SkColorGetB(color)) <= 1);
}
} // namespace content
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment