oop: Fix flashing by moving preamble to gles2_implemntation
The https://chromium-review.googlesource.com/726979 patch caused some
bugs with --enable-oop-rasterization was on. In particular, by
adding saves and restores, it wrapped all the "preamble" logic for the
first RasterCHROMIUM with the setup logic in a save/restore. This
caused any partial raster tile to be incorrect and caused flashing.
This patch moves the preamble logic into the RasterCHROMIUM call itself,
addressing a TODO in SerializeHelper. This is needed for the future
anyway so that the underlying tracking SkCanvas can have the correct
state for image decode querying.
By moving the preamble logic, saves/restores can be added to real
DrawRecords and not the fake preamble DrawRecord and the flashing bug
is also fixed.
Bug: 758350
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I64b3ee698e7712e55ed0170d1d77b62d8a87b72a
Reviewed-on: https://chromium-review.googlesource.com/745523
Commit-Queue: enne <enne@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: vmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512969}diff --git a/cc/raster/gpu_raster_buffer_provider.cc b/cc/raster/gpu_raster_buffer_provider.cc
index 7bf4d58..d10cb1b 100644
--- a/cc/raster/gpu_raster_buffer_provider.cc
+++ b/cc/raster/gpu_raster_buffer_provider.cc
@@ -30,27 +30,6 @@
namespace cc {
namespace {
-// Reuse canvas setup code from RasterSource by storing it into a PaintRecord
-// that can then be transported. This is somewhat more convoluted then it
-// should be.
-static sk_sp<PaintRecord> SetupForRaster(
- const RasterSource* raster_source,
- const gfx::Rect& raster_full_rect,
- const gfx::Rect& playback_rect,
- const gfx::AxisTransform2d& transform) {
- PaintRecorder recorder;
- PaintCanvas* canvas =
- recorder.beginRecording(gfx::RectToSkRect(raster_full_rect));
- // TODO(enne): The GLES2Decoder is guaranteeing the clear here, but it
- // might be nice to figure out how to include the debugging clears for
- // this mode.
- canvas->translate(-raster_full_rect.x(), -raster_full_rect.y());
- canvas->clipRect(SkRect::MakeFromIRect(gfx::RectToSkIRect(playback_rect)));
- canvas->translate(transform.translation().x(), transform.translation().y());
- canvas->scale(transform.scale(), transform.scale());
- return recorder.finishRecordingAsPicture();
-}
-
static void RasterizeSourceOOP(
const RasterSource* raster_source,
bool resource_has_previous_content,
@@ -66,24 +45,19 @@
gpu::gles2::GLES2Interface* gl = context_provider->ContextGL();
GLuint texture_id = resource_lock->ConsumeTexture(gl);
- auto setup_list = base::MakeRefCounted<DisplayItemList>(
- DisplayItemList::kTopLevelDisplayItemList);
- setup_list->StartPaint();
- setup_list->push<DrawRecordOp>(SetupForRaster(raster_source, raster_full_rect,
- playback_rect, transform));
- setup_list->EndPaintOfUnpaired(raster_full_rect);
- setup_list->Finalize();
-
- // TODO(enne): need to pass color space and transform in the decoder.
gl->BeginRasterCHROMIUM(texture_id, raster_source->background_color(),
msaa_sample_count, playback_settings.use_lcd_text,
use_distance_field_text,
resource_lock->PixelConfig());
- gl->RasterCHROMIUM(setup_list.get(), playback_rect.x(), playback_rect.y(),
- playback_rect.width(), playback_rect.height());
+ // TODO(enne): need to pass color space into this function as well.
+ float recording_to_raster_scale =
+ transform.scale() / raster_source->recording_scale_factor();
gl->RasterCHROMIUM(raster_source->GetDisplayItemList().get(),
+ raster_full_rect.x(), raster_full_rect.y(),
playback_rect.x(), playback_rect.y(),
- playback_rect.width(), playback_rect.height());
+ playback_rect.width(), playback_rect.height(),
+ transform.translation().x(), transform.translation().y(),
+ recording_to_raster_scale);
gl->EndRasterCHROMIUM();
gl->DeleteTextures(1, &texture_id);
diff --git a/cc/raster/raster_source.h b/cc/raster/raster_source.h
index b062627..8f14c7d5 100644
--- a/cc/raster/raster_source.h
+++ b/cc/raster/raster_source.h
@@ -111,6 +111,8 @@
return display_list_;
}
+ float recording_scale_factor() const { return recording_scale_factor_; }
+
SkColor background_color() const { return background_color_; }
base::flat_map<PaintImage::Id, PaintImage::DecodingMode>
diff --git a/gpu/command_buffer/client/gles2_c_lib_autogen.h b/gpu/command_buffer/client/gles2_c_lib_autogen.h
index 5e075f73..51632c0 100644
--- a/gpu/command_buffer/client/gles2_c_lib_autogen.h
+++ b/gpu/command_buffer/client/gles2_c_lib_autogen.h
@@ -1778,11 +1778,18 @@
use_distance_field_text, pixel_config);
}
void GL_APIENTRY GLES2RasterCHROMIUM(const cc::DisplayItemList* list,
- GLint x,
- GLint y,
- GLint w,
- GLint h) {
- gles2::GetGLContext()->RasterCHROMIUM(list, x, y, w, h);
+ GLint translate_x,
+ GLint translate_y,
+ GLint clip_x,
+ GLint clip_y,
+ GLint clip_w,
+ GLint clip_h,
+ GLfloat post_translate_x,
+ GLfloat post_translate_y,
+ GLfloat post_scale) {
+ gles2::GetGLContext()->RasterCHROMIUM(
+ list, translate_x, translate_y, clip_x, clip_y, clip_w, clip_h,
+ post_translate_x, post_translate_y, post_scale);
}
void GL_APIENTRY GLES2EndRasterCHROMIUM() {
gles2::GetGLContext()->EndRasterCHROMIUM();
diff --git a/gpu/command_buffer/client/gles2_implementation.cc b/gpu/command_buffer/client/gles2_implementation.cc
index 60e4aedc..4938680 100644
--- a/gpu/command_buffer/client/gles2_implementation.cc
+++ b/gpu/command_buffer/client/gles2_implementation.cc
@@ -52,6 +52,8 @@
#if !defined(OS_NACL)
#include "cc/paint/display_item_list.h" // nogncheck
#include "third_party/skia/include/utils/SkNoDrawCanvas.h"
+#include "ui/gfx/geometry/rect_conversions.h"
+#include "ui/gfx/skia_util.h"
#endif
#if !defined(__native_client__)
@@ -7176,6 +7178,14 @@
free_bytes_ -= size;
}
+ int Save(const cc::PlaybackParams& playback_params) {
+ int save_count = canvas_.getSaveCount();
+ cc::SaveOp save_op;
+ save_op.type = static_cast<uint8_t>(cc::PaintOpType::Save);
+ Serialize(&save_op, playback_params);
+ return save_count;
+ }
+
void RestoreTo(int count, const cc::PlaybackParams& playback_params) {
cc::RestoreOp restore_op;
restore_op.type = static_cast<uint8_t>(cc::PaintOpType::Restore);
@@ -7201,10 +7211,6 @@
cc::PaintOp::SerializeOptions& options_;
ScopedTransferBufferPtr transfer_buffer_;
GLES2CmdHelper* helper_;
- // TODO(enne): this canvas needs to persist across multiple raster calls.
- // Probably the solution here is to move the "preamble" into the raster
- // command itself so that the entire raster call + preamble happens in
- // one call to GLES2Implementation::RasterChromium.
SkNoDrawCanvas canvas_;
size_t written_bytes_ = 0;
@@ -7215,44 +7221,47 @@
const std::vector<size_t>* indices,
SerializeHelper* serializer) {
cc::PlaybackParams params(nullptr, serializer->canvas()->getTotalMatrix());
- int save_count = serializer->canvas()->getSaveCount();
for (cc::PaintOpBuffer::CompositeIterator iter(buffer, indices); iter;
++iter) {
const cc::PaintOp* op = *iter;
if (op->GetType() == cc::PaintOpType::DrawRecord) {
// TODO(enne): Add some flag here to save ctm, or adjust setmatrix ops.
- cc::SaveOp save_op;
- save_op.type = static_cast<uint8_t>(cc::PaintOpType::Save);
- serializer->Serialize(&save_op, params);
-
+ int save_count = serializer->Save(params);
SerializeRecursive(static_cast<const cc::DrawRecordOp*>(op)->record.get(),
nullptr, serializer);
-
- cc::RestoreOp restore_op;
- restore_op.type = static_cast<uint8_t>(cc::PaintOpType::Restore);
- serializer->Serialize(&restore_op, params);
- continue;
+ serializer->RestoreTo(save_count, params);
+ } else {
+ serializer->Serialize(op, params);
}
-
- serializer->Serialize(op, params);
}
-
- serializer->RestoreTo(save_count, params);
}
#endif
void GLES2Implementation::RasterCHROMIUM(const cc::DisplayItemList* list,
- GLint x,
- GLint y,
- GLint w,
- GLint h) {
+ GLint translate_x,
+ GLint translate_y,
+ GLint clip_x,
+ GLint clip_y,
+ GLint clip_w,
+ GLint clip_h,
+ GLfloat post_translate_x,
+ GLfloat post_translate_y,
+ GLfloat post_scale) {
#if defined(OS_NACL)
NOTREACHED();
#else
GPU_CLIENT_SINGLE_THREAD_CHECK();
GPU_CLIENT_LOG("[" << GetLogPrefix() << "] glRasterChromium(" << list << ", "
- << x << ", " << y << ", " << w << ", " << h << ")");
+ << translate_x << ", " << translate_y << ", " << clip_x
+ << ", " << clip_y << ", " << clip_w << ", " << clip_h
+ << ", " << post_translate_x << ", " << post_translate_y
+ << ", " << post_scale << ")");
+
+ gfx::Rect playback_rect(clip_x, clip_y, clip_w, clip_h);
+ std::vector<size_t> indices = list->rtree_.Search(playback_rect);
+ if (indices.empty())
+ return;
// TODO(enne): tune these numbers
// TODO(enne): convert these types here and in transfer buffer to be size_t.
@@ -7261,11 +7270,35 @@
cc::PaintOp::SerializeOptions options;
SerializeHelper serializer(options, free_size, transfer_buffer_, helper_);
+ // This section duplicates RasterSource::PlaybackToCanvas setup preamble.
+ cc::PlaybackParams params(nullptr, serializer.canvas()->getTotalMatrix());
+ // Bookend with save/restore so each RasterCHROMIUM command is independent.
+ int save_count = serializer.Save(params);
+
+ cc::TranslateOp translate(-translate_x, -translate_y);
+ translate.type = static_cast<uint8_t>(cc::PaintOpType::Translate);
+ serializer.Serialize(&translate, params);
+
+ cc::ClipRectOp clip(SkRect::MakeFromIRect(gfx::RectToSkIRect(playback_rect)),
+ SkClipOp::kIntersect, false);
+ clip.type = static_cast<uint8_t>(cc::PaintOpType::ClipRect);
+ serializer.Serialize(&clip, params);
+
+ if (post_translate_x != 0.f || post_translate_y != 0.f) {
+ cc::TranslateOp post_translate(post_translate_x, post_translate_y);
+ post_translate.type = static_cast<uint8_t>(cc::PaintOpType::Translate);
+ serializer.Serialize(&post_translate, params);
+ }
+ if (post_scale != 1.f) {
+ cc::ScaleOp scale(post_scale, post_scale);
+ scale.type = static_cast<uint8_t>(cc::PaintOpType::Scale);
+ serializer.Serialize(&scale, params);
+ }
+
// TODO(enne): need to implement alpha folding optimization from POB.
// TODO(enne): don't access private members of DisplayItemList.
- gfx::Rect playback_rect(x, y, w, h);
- std::vector<size_t> indices = list->rtree_.Search(playback_rect);
SerializeRecursive(&list->paint_op_buffer_, &indices, &serializer);
+ serializer.RestoreTo(save_count, params);
serializer.SendSerializedData();
CheckGLError();
diff --git a/gpu/command_buffer/client/gles2_implementation_autogen.h b/gpu/command_buffer/client/gles2_implementation_autogen.h
index 6594e53..3ccf84f 100644
--- a/gpu/command_buffer/client/gles2_implementation_autogen.h
+++ b/gpu/command_buffer/client/gles2_implementation_autogen.h
@@ -1247,10 +1247,15 @@
GLint pixel_config) override;
void RasterCHROMIUM(const cc::DisplayItemList* list,
- GLint x,
- GLint y,
- GLint w,
- GLint h) override;
+ GLint translate_x,
+ GLint translate_y,
+ GLint clip_x,
+ GLint clip_y,
+ GLint clip_w,
+ GLint clip_h,
+ GLfloat post_translate_x,
+ GLfloat post_translate_y,
+ GLfloat post_scale) override;
void EndRasterCHROMIUM() override;
diff --git a/gpu/command_buffer/client/gles2_interface_autogen.h b/gpu/command_buffer/client/gles2_interface_autogen.h
index 1d30149..e680647 100644
--- a/gpu/command_buffer/client/gles2_interface_autogen.h
+++ b/gpu/command_buffer/client/gles2_interface_autogen.h
@@ -926,10 +926,15 @@
GLboolean use_distance_field_text,
GLint pixel_config) = 0;
virtual void RasterCHROMIUM(const cc::DisplayItemList* list,
- GLint x,
- GLint y,
- GLint w,
- GLint h) = 0;
+ GLint translate_x,
+ GLint translate_y,
+ GLint clip_x,
+ GLint clip_y,
+ GLint clip_w,
+ GLint clip_h,
+ GLfloat post_translate_x,
+ GLfloat post_translate_y,
+ GLfloat post_scale) = 0;
virtual void EndRasterCHROMIUM() = 0;
virtual void TexStorage2DImageCHROMIUM(GLenum target,
GLenum internalFormat,
diff --git a/gpu/command_buffer/client/gles2_interface_stub_autogen.h b/gpu/command_buffer/client/gles2_interface_stub_autogen.h
index 9b25a5c..63cfe3f 100644
--- a/gpu/command_buffer/client/gles2_interface_stub_autogen.h
+++ b/gpu/command_buffer/client/gles2_interface_stub_autogen.h
@@ -899,10 +899,15 @@
GLboolean use_distance_field_text,
GLint pixel_config) override;
void RasterCHROMIUM(const cc::DisplayItemList* list,
- GLint x,
- GLint y,
- GLint w,
- GLint h) override;
+ GLint translate_x,
+ GLint translate_y,
+ GLint clip_x,
+ GLint clip_y,
+ GLint clip_w,
+ GLint clip_h,
+ GLfloat post_translate_x,
+ GLfloat post_translate_y,
+ GLfloat post_scale) override;
void EndRasterCHROMIUM() override;
void TexStorage2DImageCHROMIUM(GLenum target,
GLenum internalFormat,
diff --git a/gpu/command_buffer/client/gles2_interface_stub_impl_autogen.h b/gpu/command_buffer/client/gles2_interface_stub_impl_autogen.h
index c6410449..94e0aea 100644
--- a/gpu/command_buffer/client/gles2_interface_stub_impl_autogen.h
+++ b/gpu/command_buffer/client/gles2_interface_stub_impl_autogen.h
@@ -1210,10 +1210,15 @@
GLboolean /* use_distance_field_text */,
GLint /* pixel_config */) {}
void GLES2InterfaceStub::RasterCHROMIUM(const cc::DisplayItemList* /* list */,
- GLint /* x */,
- GLint /* y */,
- GLint /* w */,
- GLint /* h */) {}
+ GLint /* translate_x */,
+ GLint /* translate_y */,
+ GLint /* clip_x */,
+ GLint /* clip_y */,
+ GLint /* clip_w */,
+ GLint /* clip_h */,
+ GLfloat /* post_translate_x */,
+ GLfloat /* post_translate_y */,
+ GLfloat /* post_scale */) {}
void GLES2InterfaceStub::EndRasterCHROMIUM() {}
void GLES2InterfaceStub::TexStorage2DImageCHROMIUM(GLenum /* target */,
GLenum /* internalFormat */,
diff --git a/gpu/command_buffer/client/gles2_trace_implementation_autogen.h b/gpu/command_buffer/client/gles2_trace_implementation_autogen.h
index 2ff931f..21dc439 100644
--- a/gpu/command_buffer/client/gles2_trace_implementation_autogen.h
+++ b/gpu/command_buffer/client/gles2_trace_implementation_autogen.h
@@ -899,10 +899,15 @@
GLboolean use_distance_field_text,
GLint pixel_config) override;
void RasterCHROMIUM(const cc::DisplayItemList* list,
- GLint x,
- GLint y,
- GLint w,
- GLint h) override;
+ GLint translate_x,
+ GLint translate_y,
+ GLint clip_x,
+ GLint clip_y,
+ GLint clip_w,
+ GLint clip_h,
+ GLfloat post_translate_x,
+ GLfloat post_translate_y,
+ GLfloat post_scale) override;
void EndRasterCHROMIUM() override;
void TexStorage2DImageCHROMIUM(GLenum target,
GLenum internalFormat,
diff --git a/gpu/command_buffer/client/gles2_trace_implementation_impl_autogen.h b/gpu/command_buffer/client/gles2_trace_implementation_impl_autogen.h
index e71369e1..d6769f8 100644
--- a/gpu/command_buffer/client/gles2_trace_implementation_impl_autogen.h
+++ b/gpu/command_buffer/client/gles2_trace_implementation_impl_autogen.h
@@ -2584,12 +2584,18 @@
}
void GLES2TraceImplementation::RasterCHROMIUM(const cc::DisplayItemList* list,
- GLint x,
- GLint y,
- GLint w,
- GLint h) {
+ GLint translate_x,
+ GLint translate_y,
+ GLint clip_x,
+ GLint clip_y,
+ GLint clip_w,
+ GLint clip_h,
+ GLfloat post_translate_x,
+ GLfloat post_translate_y,
+ GLfloat post_scale) {
TRACE_EVENT_BINARY_EFFICIENT0("gpu", "GLES2Trace::RasterCHROMIUM");
- gl_->RasterCHROMIUM(list, x, y, w, h);
+ gl_->RasterCHROMIUM(list, translate_x, translate_y, clip_x, clip_y, clip_w,
+ clip_h, post_translate_x, post_translate_y, post_scale);
}
void GLES2TraceImplementation::EndRasterCHROMIUM() {
diff --git a/gpu/command_buffer/cmd_buffer_functions.txt b/gpu/command_buffer/cmd_buffer_functions.txt
index 32567a5..87cbc39 100644
--- a/gpu/command_buffer/cmd_buffer_functions.txt
+++ b/gpu/command_buffer/cmd_buffer_functions.txt
@@ -379,7 +379,7 @@
// Extension CHROMIUM_raster_transport
GL_APICALL void GL_APIENTRY glBeginRasterCHROMIUM (GLuint texture_id, GLuint sk_color, GLuint msaa_sample_count, GLboolean can_use_lcd_text, GLboolean use_distance_field_text, GLint pixel_config);
-GL_APICALL void GL_APIENTRY glRasterCHROMIUM (const cc::DisplayItemList* list, GLint x, GLint y, GLint w, GLint h);
+GL_APICALL void GL_APIENTRY glRasterCHROMIUM (const cc::DisplayItemList* list, GLint translate_x, GLint translate_y, GLint clip_x, GLint clip_y, GLint clip_w, GLint clip_h, GLfloat post_translate_x, GLfloat post_translate_y, GLfloat post_scale);
GL_APICALL void GL_APIENTRY glEndRasterCHROMIUM (void);
// Extension CHROMIUM_texture_storage_image