Skip to content

Commit

Permalink
Revert "Reland "Run Flutter on iOS and Android with color correct Ski…
Browse files Browse the repository at this point in the history
…a" (flutter#3818)" (flutter#3823)

This reverts commit 2650f52.
  • Loading branch information
brianosman authored Jun 24, 2017
1 parent 2650f52 commit db8d8a9
Show file tree
Hide file tree
Showing 19 changed files with 94 additions and 196 deletions.
2 changes: 1 addition & 1 deletion flow/compositor_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class CompositorContext {
public:
class ScopedFrame {
public:
SkCanvas* canvas() { return canvas_; }
SkCanvas& canvas() { return *canvas_; }

CompositorContext& context() const { return context_; }

Expand Down
1 change: 0 additions & 1 deletion flow/layers/layer.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ class Layer {
struct PrerollContext {
RasterCache* raster_cache;
GrContext* gr_context;
SkColorSpace* dst_color_space;
SkRect child_paint_bounds;
};

Expand Down
6 changes: 2 additions & 4 deletions flow/layers/layer_tree.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,11 @@ void LayerTree::Raster(CompositorContext::ScopedFrame& frame,
void LayerTree::Preroll(CompositorContext::ScopedFrame& frame,
bool ignore_raster_cache) {
TRACE_EVENT0("flutter", "LayerTree::Preroll");
SkColorSpace* color_space =
frame.canvas() ? frame.canvas()->imageInfo().colorSpace() : nullptr;
frame.context().raster_cache().SetCheckboardCacheImages(
checkerboard_raster_cache_images_);
Layer::PrerollContext context = {
ignore_raster_cache ? nullptr : &frame.context().raster_cache(),
frame.gr_context(), color_space, SkRect::MakeEmpty(),
frame.gr_context(), SkRect::MakeEmpty(),
};
root_layer_->Preroll(&context, SkMatrix::I());
}
Expand All @@ -53,7 +51,7 @@ void LayerTree::UpdateScene(SceneUpdateContext& context,
#endif

void LayerTree::Paint(CompositorContext::ScopedFrame& frame) {
Layer::PaintContext context = {*frame.canvas(), frame.context().frame_time(),
Layer::PaintContext context = {frame.canvas(), frame.context().frame_time(),
frame.context().engine_time(),
frame.context().memory_usage(),
checkerboard_offscreen_layers_};
Expand Down
3 changes: 1 addition & 2 deletions flow/layers/picture_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ PictureLayer::~PictureLayer() {
void PictureLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) {
if (auto cache = context->raster_cache) {
raster_cache_result_ = cache->GetPrerolledImage(
context->gr_context, picture_.get(), matrix, context->dst_color_space,
is_complex_, will_change_);
context->gr_context, picture_.get(), matrix, is_complex_, will_change_);
}

SkRect bounds = picture_->cullRect().makeOffset(offset_.x(), offset_.y());
Expand Down
7 changes: 2 additions & 5 deletions flow/raster_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ static bool IsPictureWorthRasterizing(SkPicture* picture,
RasterCacheResult RasterizePicture(SkPicture* picture,
GrContext* context,
const MatrixDecomposition& matrix,
SkColorSpace* dst_color_space,
bool checkerboard) {
TRACE_EVENT0("flutter", "RasterCachePopulate");

Expand All @@ -82,7 +81,7 @@ RasterCacheResult RasterizePicture(SkPicture* picture,
std::ceil(logical_rect.width() * std::abs(scale.x())), // physical width
std::ceil(logical_rect.height() *
std::abs(scale.y())), // physical height
sk_ref_sp(dst_color_space) // colorspace
nullptr // colorspace
);

sk_sp<SkSurface> surface =
Expand Down Expand Up @@ -131,7 +130,6 @@ RasterCacheResult RasterCache::GetPrerolledImage(
GrContext* context,
SkPicture* picture,
const SkMatrix& transformation_matrix,
SkColorSpace* dst_color_space,
bool is_complex,
bool will_change) {
if (!IsPictureWorthRasterizing(picture, will_change, is_complex)) {
Expand Down Expand Up @@ -161,8 +159,7 @@ RasterCacheResult RasterCache::GetPrerolledImage(

if (!entry.image.is_valid()) {
entry.image =
RasterizePicture(picture, context, matrix, dst_color_space,
checkerboard_images_);
RasterizePicture(picture, context, matrix, checkerboard_images_);
}

// We are not considering unrasterizable images. So if we don't have an image
Expand Down
2 changes: 0 additions & 2 deletions flow/raster_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,8 @@ class RasterCache {
RasterCacheResult GetPrerolledImage(GrContext* context,
SkPicture* picture,
const SkMatrix& transformation_matrix,
SkColorSpace* dst_color_space,
bool is_complex,
bool will_change);

void SweepAfterFrame();

void Clear();
Expand Down
25 changes: 11 additions & 14 deletions flow/raster_cache_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,14 @@ TEST(RasterCache, ThresholdIsRespected) {

sk_sp<SkImage> image;

sk_sp<SkColorSpace> srgb = SkColorSpace::MakeSRGB();
ASSERT_FALSE(
cache.GetPrerolledImage(NULL, picture.get(), matrix, srgb, true, false)); // 1
cache.GetPrerolledImage(NULL, picture.get(), matrix, true, false)); // 1
cache.SweepAfterFrame();
ASSERT_FALSE(
cache.GetPrerolledImage(NULL, picture.get(), matrix, srgb, true, false)); // 2
cache.GetPrerolledImage(NULL, picture.get(), matrix, true, false)); // 2
cache.SweepAfterFrame();
ASSERT_TRUE(
cache.GetPrerolledImage(NULL, picture.get(), matrix, srgb, true, false)); // 3
cache.GetPrerolledImage(NULL, picture.get(), matrix, true, false)); // 3
cache.SweepAfterFrame();
}

Expand All @@ -54,15 +53,14 @@ TEST(RasterCache, ThresholdIsRespectedWhenZero) {

sk_sp<SkImage> image;

sk_sp<SkColorSpace> srgb = SkColorSpace::MakeSRGB();
ASSERT_FALSE(
cache.GetPrerolledImage(NULL, picture.get(), matrix, srgb, true, false)); // 1
cache.GetPrerolledImage(NULL, picture.get(), matrix, true, false)); // 1
cache.SweepAfterFrame();
ASSERT_FALSE(
cache.GetPrerolledImage(NULL, picture.get(), matrix, srgb, true, false)); // 2
cache.GetPrerolledImage(NULL, picture.get(), matrix, true, false)); // 2
cache.SweepAfterFrame();
ASSERT_FALSE(
cache.GetPrerolledImage(NULL, picture.get(), matrix, srgb, true, false)); // 3
cache.GetPrerolledImage(NULL, picture.get(), matrix, true, false)); // 3
cache.SweepAfterFrame();
}

Expand All @@ -76,20 +74,19 @@ TEST(RasterCache, SweepsRemoveUnusedFrames) {

sk_sp<SkImage> image;

sk_sp<SkColorSpace> srgb = SkColorSpace::MakeSRGB();
ASSERT_FALSE(
cache.GetPrerolledImage(NULL, picture.get(), matrix, srgb, true, false)); // 1
cache.GetPrerolledImage(NULL, picture.get(), matrix, true, false)); // 1
cache.SweepAfterFrame();
ASSERT_FALSE(
cache.GetPrerolledImage(NULL, picture.get(), matrix, srgb, true, false)); // 2
cache.GetPrerolledImage(NULL, picture.get(), matrix, true, false)); // 2
cache.SweepAfterFrame();
ASSERT_TRUE(
cache.GetPrerolledImage(NULL, picture.get(), matrix, srgb, true, false)); // 3
cache.GetPrerolledImage(NULL, picture.get(), matrix, true, false)); // 3
cache.SweepAfterFrame();
ASSERT_TRUE(
cache.GetPrerolledImage(NULL, picture.get(), matrix, srgb, true, false)); // 4
cache.GetPrerolledImage(NULL, picture.get(), matrix, true, false)); // 4
cache.SweepAfterFrame();
cache.SweepAfterFrame(); // Extra frame without a preroll image access.
ASSERT_FALSE(
cache.GetPrerolledImage(NULL, picture.get(), matrix, srgb, true, false)); // 5
cache.GetPrerolledImage(NULL, picture.get(), matrix, true, false)); // 5
}
5 changes: 2 additions & 3 deletions lib/ui/painting/image_decoding.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,9 @@ sk_sp<SkImage> DecodeImage(sk_sp<SkData> buffer) {

GrContext* context = ResourceContext::Get();
if (context) {
// This acts as a flag to indicate that we want a color space aware decode.
sk_sp<SkColorSpace> dstColorSpace = SkColorSpace::MakeSRGB();
// TODO: Supply actual destination color space once available
return SkImage::MakeCrossContextFromEncoded(context, std::move(buffer),
false, dstColorSpace.get());
false, nullptr);
} else {
return SkImage::MakeFromEncoded(std::move(buffer));
}
Expand Down
1 change: 0 additions & 1 deletion shell/common/platform_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,6 @@ void PlatformView::SetupResourceContextOnIOThreadPerform(
// other threads correctly, so the textures end up blank. For now, suppress
// that feature, which will cause texture uploads to do CPU YUV conversion.
options.fDisableGpuYUVConversion = true;
options.fRequireDecodeDisableForSRGB = false;

blink::ResourceContext::Set(GrContext::Create(
GrBackend::kOpenGL_GrBackend,
Expand Down
36 changes: 12 additions & 24 deletions shell/gpu/gpu_surface_gl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include "lib/ftl/arraysize.h"
#include "lib/ftl/logging.h"
#include "third_party/skia/include/core/SkSurface.h"
#include "third_party/skia/include/gpu/GrContextOptions.h"
#include "third_party/skia/include/gpu/gl/GrGLInterface.h"

namespace shell {
Expand Down Expand Up @@ -45,12 +44,9 @@ bool GPUSurfaceGL::Setup() {
auto backend_context =
reinterpret_cast<GrBackendContext>(GrGLCreateNativeInterface());

GrContextOptions options;
options.fRequireDecodeDisableForSRGB = false;

context_ =
sk_sp<GrContext>(GrContext::Create(kOpenGL_GrBackend, backend_context,
options));
sk_sp<GrContext>(GrContext::Create(kOpenGL_GrBackend, backend_context));

if (context_ == nullptr) {
FTL_LOG(INFO) << "Failed to setup Skia Gr context.";
return false;
Expand Down Expand Up @@ -108,23 +104,15 @@ bool GPUSurfaceGL::PresentSurface(SkCanvas* canvas) {
}

bool GPUSurfaceGL::SelectPixelConfig(GrPixelConfig* config) {
if (delegate_->ColorSpace() && delegate_->ColorSpace()->gammaCloseToSRGB()) {
FTL_DCHECK(context_->caps()->isConfigRenderable(kSRGBA_8888_GrPixelConfig,
false));
*config = kSRGBA_8888_GrPixelConfig;
return true;
}

// FIXME:
// If sRGB support is not available, we should instead fall back to software.
if (context_->caps()->isConfigRenderable(kRGBA_8888_GrPixelConfig, false)) {
*config = kRGBA_8888_GrPixelConfig;
return true;
}
static const GrPixelConfig kConfigOptions[] = {
kSkia8888_GrPixelConfig, kRGBA_4444_GrPixelConfig,
};

if (context_->caps()->isConfigRenderable(kRGBA_4444_GrPixelConfig, false)) {
*config = kRGBA_4444_GrPixelConfig;
return true;
for (size_t i = 0; i < arraysize(kConfigOptions); i++) {
if (context_->caps()->isConfigRenderable(kConfigOptions[i], false)) {
*config = kConfigOptions[i];
return true;
}
}

return false;
Expand All @@ -136,6 +124,7 @@ sk_sp<SkSurface> GPUSurfaceGL::CreateSurface(const SkISize& size) {
}

GrBackendRenderTargetDesc desc;

if (!SelectPixelConfig(&desc.fConfig)) {
return nullptr;
}
Expand All @@ -146,8 +135,7 @@ sk_sp<SkSurface> GPUSurfaceGL::CreateSurface(const SkISize& size) {
desc.fOrigin = kBottomLeft_GrSurfaceOrigin;
desc.fRenderTargetHandle = delegate_->GLContextFBO();

return SkSurface::MakeFromBackendRenderTarget(context_.get(), desc,
delegate_->ColorSpace(), nullptr);
return SkSurface::MakeFromBackendRenderTarget(context_.get(), desc, nullptr);
}

sk_sp<SkSurface> GPUSurfaceGL::AcquireSurface(const SkISize& size) {
Expand Down
3 changes: 0 additions & 3 deletions shell/gpu/gpu_surface_gl.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,6 @@ class GPUSurfaceGLDelegate {
virtual bool GLContextPresent() = 0;

virtual intptr_t GLContextFBO() const = 0;

// TODO: Update Mac desktop and make this pure virtual.
virtual sk_sp<SkColorSpace> ColorSpace() const { return nullptr; }
};

class GPUSurfaceGL : public Surface {
Expand Down
Loading

0 comments on commit db8d8a9

Please sign in to comment.