Skip to content

Commit

Permalink
Revert "Revert "Reland "Run Flutter on iOS and Android with color cor…
Browse files Browse the repository at this point in the history
…rect Skia (flutter#3826)" (flutter#3878)" (flutter#3895)

This reverts commit 1db18a4.
  • Loading branch information
brianosman authored Jul 18, 2017
1 parent cfa180d commit de00757
Show file tree
Hide file tree
Showing 19 changed files with 198 additions and 96 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: 1 addition & 0 deletions flow/layers/layer.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ class Layer {
struct PrerollContext {
RasterCache* raster_cache;
GrContext* gr_context;
SkColorSpace* dst_color_space;
SkRect child_paint_bounds;
};

Expand Down
6 changes: 4 additions & 2 deletions flow/layers/layer_tree.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,13 @@ 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(), SkRect::MakeEmpty(),
frame.gr_context(), color_space, SkRect::MakeEmpty(),
};
root_layer_->Preroll(&context, SkMatrix::I());
}
Expand All @@ -51,7 +53,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: 2 additions & 1 deletion flow/layers/picture_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ 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, is_complex_, will_change_);
context->gr_context, picture_.get(), matrix, context->dst_color_space,
is_complex_, will_change_);
}

SkRect bounds = picture_->cullRect().makeOffset(offset_.x(), offset_.y());
Expand Down
11 changes: 7 additions & 4 deletions flow/raster_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ 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,9 +83,9 @@ RasterCacheResult RasterizePicture(SkPicture* picture,
std::ceil(std::fabs(logical_rect.height() * scale.y())));

const SkImageInfo image_info =
SkImageInfo::MakeN32Premul(physical_rect.width(), // physical width
physical_rect.height(), // physical height
nullptr // colorspace
SkImageInfo::MakeN32Premul(physical_rect.width(), // physical width
physical_rect.height(), // physical height
sk_ref_sp(dst_color_space) // colorspace
);

sk_sp<SkSurface> surface =
Expand Down Expand Up @@ -131,6 +132,7 @@ 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 @@ -160,7 +162,8 @@ RasterCacheResult RasterCache::GetPrerolledImage(

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

// We are not considering unrasterizable images. So if we don't have an image
Expand Down
2 changes: 2 additions & 0 deletions flow/raster_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,10 @@ 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: 14 additions & 11 deletions flow/raster_cache_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,15 @@ TEST(RasterCache, ThresholdIsRespected) {

sk_sp<SkImage> image;

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

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

sk_sp<SkImage> image;

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

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

sk_sp<SkImage> image;

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

GrContext* context = ResourceContext::Get();
if (context) {
// TODO: Supply actual destination color space once available
// This acts as a flag to indicate that we want a color space aware decode.
sk_sp<SkColorSpace> dstColorSpace = SkColorSpace::MakeSRGB();
return SkImage::MakeCrossContextFromEncoded(context, std::move(buffer),
false, nullptr);
false, dstColorSpace.get());
} else {
return SkImage::MakeFromEncoded(std::move(buffer));
}
Expand Down
1 change: 1 addition & 0 deletions shell/common/platform_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ 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: 24 additions & 12 deletions shell/gpu/gpu_surface_gl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#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 @@ -44,9 +45,12 @@ bool GPUSurfaceGL::Setup() {
auto backend_context =
reinterpret_cast<GrBackendContext>(GrGLCreateNativeInterface());

context_ =
sk_sp<GrContext>(GrContext::Create(kOpenGL_GrBackend, backend_context));
GrContextOptions options;
options.fRequireDecodeDisableForSRGB = false;

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

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

for (size_t i = 0; i < arraysize(kConfigOptions); i++) {
if (context_->caps()->isConfigRenderable(kConfigOptions[i], false)) {
*config = kConfigOptions[i];
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;
}

if (context_->caps()->isConfigRenderable(kRGBA_4444_GrPixelConfig, false)) {
*config = kRGBA_4444_GrPixelConfig;
return true;
}

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

GrBackendRenderTargetDesc desc;

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

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

sk_sp<SkSurface> GPUSurfaceGL::AcquireSurface(const SkISize& size) {
Expand Down
3 changes: 3 additions & 0 deletions shell/gpu/gpu_surface_gl.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ 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 de00757

Please sign in to comment.