Skip to content

Commit

Permalink
color: Enable color correct rendering on all cc/viz tests
Browse files Browse the repository at this point in the history 
Fix a couple of bugs along the way, mostly where we didn't specify a
color space for YUV quads.

Bug: 759215
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I2490ff526ffa3f1c8841cf0751a641e0c9d1f835
Reviewed-on: https://chromium-review.googlesource.com/636170
Commit-Queue: ccameron chromium <ccameron@chromium.org>
Reviewed-by: Vladimir Levin <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499530}
  • Loading branch information
@ccameron-chromium
ccameron-chromium authored and Commit Bot committed Sep 5, 2017
1 parent 2132849 commit 7fa8e28
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 35 deletions.
34 changes: 20 additions & 14 deletions cc/output/renderer_pixeltest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ void CreateTestYUVVideoDrawQuad_FromVideoFrame(
}

gfx::ColorSpace video_color_space = video_frame->ColorSpace();
DCHECK(video_color_space.IsValid());

bool needs_blending = true;

Expand Down Expand Up @@ -386,6 +387,7 @@ scoped_refptr<media::VideoFrame> CreateHighbitVideoFrame(
void CreateTestYUVVideoDrawQuad_Striped(
const SharedQuadState* shared_state,
media::VideoPixelFormat format,
media::ColorSpace color_space,
bool is_transparent,
bool highbit,
const gfx::RectF& tex_coord_rect,
Expand Down Expand Up @@ -425,6 +427,8 @@ void CreateTestYUVVideoDrawQuad_Striped(

if (highbit)
video_frame = CreateHighbitVideoFrame(video_frame.get());
video_frame->metadata()->SetInteger(media::VideoFrameMetadata::COLOR_SPACE,
color_space);

CreateTestYUVVideoDrawQuad_FromVideoFrame(
shared_state, video_frame, alpha_value, tex_coord_rect, render_pass,
Expand Down Expand Up @@ -548,8 +552,10 @@ void CreateTestYUVVideoDrawQuad_NV12(const SharedQuadState* shared_state,
const gfx::Rect& visible_rect,
ResourceProvider* resource_provider) {
YUVVideoDrawQuad::ColorSpace color_space = YUVVideoDrawQuad::REC_601;
gfx::ColorSpace gfx_color_space = gfx::ColorSpace::CreateREC601();
if (video_frame_color_space == media::COLOR_SPACE_JPEG) {
color_space = YUVVideoDrawQuad::JPEG;
gfx_color_space = gfx::ColorSpace::CreateJpeg();
}

bool needs_blending = true;
Expand All @@ -559,10 +565,10 @@ void CreateTestYUVVideoDrawQuad_NV12(const SharedQuadState* shared_state,

viz::ResourceId y_resource = resource_provider->CreateResource(
rect.size(), ResourceProvider::TEXTURE_HINT_DEFAULT,
resource_provider->YuvResourceFormat(8), gfx::ColorSpace());
resource_provider->YuvResourceFormat(8), gfx_color_space);
viz::ResourceId u_resource = resource_provider->CreateResource(
uv_tex_size, ResourceProvider::TEXTURE_HINT_DEFAULT, viz::RGBA_8888,
gfx::ColorSpace());
gfx_color_space);
viz::ResourceId v_resource = u_resource;
viz::ResourceId a_resource = 0;

Expand Down Expand Up @@ -1310,8 +1316,8 @@ TEST_P(VideoGLRendererPixelHiLoTest, SimpleYUVRect) {

bool highbit = GetParam();
CreateTestYUVVideoDrawQuad_Striped(
shared_state, media::PIXEL_FORMAT_YV12, false, highbit,
gfx::RectF(0.0f, 0.0f, 1.0f, 1.0f), pass.get(),
shared_state, media::PIXEL_FORMAT_YV12, media::COLOR_SPACE_SD_REC601,
false, highbit, gfx::RectF(0.0f, 0.0f, 1.0f, 1.0f), pass.get(),
video_resource_updater_.get(), rect, rect, resource_provider_.get());

RenderPassList pass_list;
Expand All @@ -1336,8 +1342,8 @@ TEST_P(VideoGLRendererPixelHiLoTest, ClippedYUVRect) {

bool highbit = GetParam();
CreateTestYUVVideoDrawQuad_Striped(
shared_state, media::PIXEL_FORMAT_YV12, false, highbit,
gfx::RectF(0.0f, 0.0f, 1.0f, 1.0f), pass.get(),
shared_state, media::PIXEL_FORMAT_YV12, media::COLOR_SPACE_SD_REC601,
false, highbit, gfx::RectF(0.0f, 0.0f, 1.0f, 1.0f), pass.get(),
video_resource_updater_.get(), draw_rect, viewport,
resource_provider_.get());
RenderPassList pass_list;
Expand All @@ -1359,8 +1365,8 @@ TEST_F(VideoGLRendererPixelHiLoTest, OffsetYUVRect) {

// Intentionally sets frame format to I420 for testing coverage.
CreateTestYUVVideoDrawQuad_Striped(
shared_state, media::PIXEL_FORMAT_I420, false, false,
gfx::RectF(0.125f, 0.25f, 0.75f, 0.5f), pass.get(),
shared_state, media::PIXEL_FORMAT_I420, media::COLOR_SPACE_SD_REC601,
false, false, gfx::RectF(0.125f, 0.25f, 0.75f, 0.5f), pass.get(),
video_resource_updater_.get(), rect, rect, resource_provider_.get());

RenderPassList pass_list;
Expand All @@ -1382,7 +1388,7 @@ TEST_F(VideoGLRendererPixelTest, SimpleYUVRectBlack) {

// In MPEG color range YUV values of (15,128,128) should produce black.
CreateTestYUVVideoDrawQuad_Solid(
shared_state, media::PIXEL_FORMAT_YV12, media::COLOR_SPACE_UNSPECIFIED,
shared_state, media::PIXEL_FORMAT_YV12, media::COLOR_SPACE_SD_REC601,
false, gfx::RectF(0.0f, 0.0f, 1.0f, 1.0f), 15, 128, 128, pass.get(),
video_resource_updater_.get(), rect, rect, resource_provider_.get());

Expand Down Expand Up @@ -1458,7 +1464,7 @@ TEST_F(VideoGLRendererPixelTest, YUVEdgeBleed) {

TEST_F(VideoGLRendererPixelTest, YUVAEdgeBleed) {
RenderPassList pass_list;
CreateEdgeBleedPass(media::PIXEL_FORMAT_YV12A, media::COLOR_SPACE_UNSPECIFIED,
CreateEdgeBleedPass(media::PIXEL_FORMAT_YV12A, media::COLOR_SPACE_SD_REC601,
&pass_list);
EXPECT_TRUE(this->RunPixelTest(&pass_list,
base::FilePath(FILE_PATH_LITERAL("green.png")),
Expand Down Expand Up @@ -1499,8 +1505,8 @@ TEST_F(VideoGLRendererPixelHiLoTest, SimpleYUVARect) {
CreateTestSharedQuadState(gfx::Transform(), rect, pass.get());

CreateTestYUVVideoDrawQuad_Striped(
shared_state, media::PIXEL_FORMAT_YV12A, false, false,
gfx::RectF(0.0f, 0.0f, 1.0f, 1.0f), pass.get(),
shared_state, media::PIXEL_FORMAT_YV12A, media::COLOR_SPACE_SD_REC601,
false, false, gfx::RectF(0.0f, 0.0f, 1.0f, 1.0f), pass.get(),
video_resource_updater_.get(), rect, rect, resource_provider_.get());

SolidColorDrawQuad* color_quad =
Expand All @@ -1526,8 +1532,8 @@ TEST_F(VideoGLRendererPixelTest, FullyTransparentYUVARect) {
CreateTestSharedQuadState(gfx::Transform(), rect, pass.get());

CreateTestYUVVideoDrawQuad_Striped(
shared_state, media::PIXEL_FORMAT_YV12A, true, false,
gfx::RectF(0.0f, 0.0f, 1.0f, 1.0f), pass.get(),
shared_state, media::PIXEL_FORMAT_YV12A, media::COLOR_SPACE_SD_REC601,
true, false, gfx::RectF(0.0f, 0.0f, 1.0f, 1.0f), pass.get(),
video_resource_updater_.get(), rect, rect, resource_provider_.get());

SolidColorDrawQuad* color_quad =
Expand Down
19 changes: 10 additions & 9 deletions cc/test/render_pass_test_utils.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,32 +124,32 @@ void AddOneOfEveryQuadType(RenderPass* to_pass,

viz::ResourceId resource1 = resource_provider->CreateResource(
gfx::Size(45, 5), ResourceProvider::TEXTURE_HINT_IMMUTABLE,
resource_provider->best_texture_format(), gfx::ColorSpace());
resource_provider->best_texture_format(), gfx::ColorSpace::CreateSRGB());
resource_provider->AllocateForTesting(resource1);
viz::ResourceId resource2 = resource_provider->CreateResource(
gfx::Size(346, 61), ResourceProvider::TEXTURE_HINT_IMMUTABLE,
resource_provider->best_texture_format(), gfx::ColorSpace());
resource_provider->best_texture_format(), gfx::ColorSpace::CreateSRGB());
resource_provider->AllocateForTesting(resource2);
viz::ResourceId resource3 = resource_provider->CreateResource(
gfx::Size(12, 134), ResourceProvider::TEXTURE_HINT_IMMUTABLE,
resource_provider->best_texture_format(), gfx::ColorSpace());
resource_provider->best_texture_format(), gfx::ColorSpace::CreateSRGB());
resource_provider->AllocateForTesting(resource3);
viz::ResourceId resource4 = resource_provider->CreateResource(
gfx::Size(56, 12), ResourceProvider::TEXTURE_HINT_IMMUTABLE,
resource_provider->best_texture_format(), gfx::ColorSpace());
resource_provider->best_texture_format(), gfx::ColorSpace::CreateSRGB());
resource_provider->AllocateForTesting(resource4);
gfx::Size resource5_size(73, 26);
viz::ResourceId resource5 = resource_provider->CreateResource(
resource5_size, ResourceProvider::TEXTURE_HINT_IMMUTABLE,
resource_provider->best_texture_format(), gfx::ColorSpace());
resource_provider->best_texture_format(), gfx::ColorSpace::CreateSRGB());
resource_provider->AllocateForTesting(resource5);
viz::ResourceId resource6 = resource_provider->CreateResource(
gfx::Size(64, 92), ResourceProvider::TEXTURE_HINT_IMMUTABLE,
resource_provider->best_texture_format(), gfx::ColorSpace());
resource_provider->best_texture_format(), gfx::ColorSpace::CreateSRGB());
resource_provider->AllocateForTesting(resource6);
viz::ResourceId resource7 = resource_provider->CreateResource(
gfx::Size(9, 14), ResourceProvider::TEXTURE_HINT_IMMUTABLE,
resource_provider->best_texture_format(), gfx::ColorSpace());
resource_provider->best_texture_format(), gfx::ColorSpace::CreateSRGB());
resource_provider->AllocateForTesting(resource7);

unsigned target = GL_TEXTURE_2D;
Expand Down Expand Up @@ -238,7 +238,8 @@ void AddOneOfEveryQuadType(RenderPass* to_pass,
for (int i = 0; i < 4; ++i) {
plane_resources[i] = resource_provider->CreateResource(
gfx::Size(20, 12), ResourceProvider::TEXTURE_HINT_IMMUTABLE,
resource_provider->best_texture_format(), gfx::ColorSpace());
resource_provider->best_texture_format(),
gfx::ColorSpace::CreateREC601());
resource_provider->AllocateForTesting(plane_resources[i]);
}
YUVVideoDrawQuad::ColorSpace color_space = YUVVideoDrawQuad::REC_601;
Expand All @@ -250,7 +251,7 @@ void AddOneOfEveryQuadType(RenderPass* to_pass,
gfx::RectF(.0f, .0f, 50.0f, 50.0f), gfx::Size(100, 100),
gfx::Size(50, 50), plane_resources[0], plane_resources[1],
plane_resources[2], plane_resources[3], color_space,
gfx::ColorSpace::CreateJpeg(), 0.0, 1.0, 8);
gfx::ColorSpace::CreateREC601(), 0.0, 1.0, 8);
}

} // namespace cc
2 changes: 1 addition & 1 deletion cc/trees/layer_tree_host_impl_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12978,13 +12978,13 @@ TEST_F(LayerTreeHostImplTest, CheckerImagingTileInvalidation) {

TEST_F(LayerTreeHostImplTest, RasterColorSpaceNoColorCorrection) {
LayerTreeSettings settings = DefaultSettings();
settings.enable_color_correct_rasterization = false;
CreateHostImpl(settings, CreateLayerTreeFrameSink());
EXPECT_FALSE(host_impl_->GetRasterColorSpace().IsValid());
}

TEST_F(LayerTreeHostImplTest, RasterColorSpace) {
LayerTreeSettings settings = DefaultSettings();
settings.enable_color_correct_rasterization = true;
CreateHostImpl(settings, CreateLayerTreeFrameSink());
// The default raster color space should be sRGB.
EXPECT_EQ(host_impl_->GetRasterColorSpace(), gfx::ColorSpace::CreateSRGB());
Expand Down
2 changes: 1 addition & 1 deletion cc/trees/layer_tree_settings.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ class CC_EXPORT LayerTreeSettings {
int max_preraster_distance_in_screen_pixels = 1000;
viz::ResourceFormat preferred_tile_format;

bool enable_color_correct_rasterization = false;
bool enable_color_correct_rasterization = true;

// TODO(sunxd): remove this flag when filter demoting and aa of mask layers
// are implemented.
Expand Down
2 changes: 1 addition & 1 deletion components/viz/common/display/renderer_settings.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class VIZ_COMMON_EXPORT RendererSettings {
bool release_overlay_resources_after_gpu_query = false;
bool gl_composited_overlay_candidate_quad_border = false;
bool show_overdraw_feedback = false;
bool enable_color_correct_rendering = false;
bool enable_color_correct_rendering = true;
bool use_skia_renderer = false;
int highp_threshold_min = 0;

Expand Down
20 changes: 11 additions & 9 deletions components/viz/service/display/gl_renderer.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2143,24 +2143,26 @@ void GLRenderer::DrawYUVVideoQuad(const cc::YUVVideoDrawQuad* quad,
break;
}
}
// Invalid or unspecified color spaces should be treated as REC709.
if (!src_color_space.IsValid())
src_color_space = gfx::ColorSpace::CreateREC709();

// The source color space should never be RGB.
DCHECK_NE(src_color_space, src_color_space.GetAsFullRangeRGB());

cc::DisplayResourceProvider::ScopedSamplerGL y_plane_lock(
resource_provider_, quad->y_plane_resource_id(), GL_TEXTURE1, GL_LINEAR);
if (base::FeatureList::IsEnabled(media::kVideoColorManagement))
DCHECK_EQ(src_color_space, y_plane_lock.color_space());
cc::DisplayResourceProvider::ScopedSamplerGL u_plane_lock(
resource_provider_, quad->u_plane_resource_id(), GL_TEXTURE2, GL_LINEAR);
DCHECK_EQ(y_plane_lock.target(), u_plane_lock.target());
DCHECK_EQ(y_plane_lock.color_space(), u_plane_lock.color_space());
// TODO(jbauman): Use base::Optional when available.
std::unique_ptr<cc::DisplayResourceProvider::ScopedSamplerGL> v_plane_lock;

// Invalid or unspecified color spaces should be treated as REC709.
if (!src_color_space.IsValid())
src_color_space = gfx::ColorSpace::CreateREC709();
#if DCHECK_IS_ON()
else if (base::FeatureList::IsEnabled(media::kVideoColorManagement))
DCHECK_EQ(src_color_space, y_plane_lock.color_space());
#endif

// The source color space should never be RGB.
DCHECK_NE(src_color_space, src_color_space.GetAsFullRangeRGB());

if (uv_texture_mode == UV_TEXTURE_MODE_U_V) {
v_plane_lock.reset(new cc::DisplayResourceProvider::ScopedSamplerGL(
resource_provider_, quad->v_plane_resource_id(), GL_TEXTURE3,
Expand Down

0 comments on commit 7fa8e28

Please sign in to comment.