Commit 779dea58 authored by Christopher Grant's avatar Christopher Grant Committed by Commit Bot

VR: Fix text updates due to user input

The omnibox is a text element that updates according to user input.
User input happens after element layout, and since the text texture
heavily relies on layout, "dirtying" the text texture really means
laying it out again.

Prior to recent cleanup and refactoring, we mistakenly got away with
this - a text element could resize itself after the layout stage and
appear incorrectly.  Post-cleanup, the re-layout can't happen, but a new
bug was introduced where a post-layout phase could dirty a texture, and
the texture update phase would redraw and "clean" the texture without
doing layout.  The broken layout would then persist indefinitely.

To address the problem, elements may mark themselves as requiring
measurement for texture draw.  These elements will not be redrawn unless
their measurement step has happened.

There was a measured() DCHECK mechanism previously, that was taken out
because most textures no longer measure themselves.  This change
reintroduces that check in it's new form.

BUG=834297

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_vr
Change-Id: Ib17315a5c053d487edfbf7b858c2e9541924c3a6
Reviewed-on: https://chromium-review.googlesource.com/1016064Reviewed-by: default avatarIan Vollick <vollick@chromium.org>
Commit-Queue: Christopher Grant <cjgrant@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551714}
parent a94af050
......@@ -104,6 +104,10 @@ UiTexture* Prompt::GetTexture() const {
return texture_.get();
}
bool Prompt::TextureDependsOnMeasurement() const {
return false;
}
gfx::Size Prompt::MeasureTextureSize() {
return texture_->GetPreferredTextureSize(preferred_width_);
}
......
......@@ -58,6 +58,7 @@ class Prompt : public TexturedElement {
private:
void OnStateUpdated(const gfx::PointF& position);
bool TextureDependsOnMeasurement() const override;
gfx::Size MeasureTextureSize() override;
bool primary_down_ = false;
......
......@@ -121,6 +121,10 @@ UiTexture* Spinner::GetTexture() const {
return texture_.get();
}
bool Spinner::TextureDependsOnMeasurement() const {
return false;
}
gfx::Size Spinner::MeasureTextureSize() {
return gfx::Size(texture_width_, texture_width_);
}
......
......@@ -26,6 +26,7 @@ class Spinner : public TexturedElement {
UiTexture* GetTexture() const override;
private:
bool TextureDependsOnMeasurement() const override;
gfx::Size MeasureTextureSize() override;
void NotifyClientFloatAnimated(float value,
......
......@@ -313,6 +313,10 @@ UiTexture* Text::GetTexture() const {
return texture_.get();
}
bool Text::TextureDependsOnMeasurement() const {
return true;
}
gfx::Size Text::MeasureTextureSize() {
text_texture_size_ = texture_->LayOutText();
......@@ -393,6 +397,8 @@ gfx::Size TextTexture::LayOutText() {
gfx::ToFlooredInt(parameters.shadow_size));
}
set_measured();
return text_bounds.size();
}
......
......@@ -127,6 +127,7 @@ class Text : public TexturedElement {
private:
UiTexture* GetTexture() const override;
bool TextureDependsOnMeasurement() const override;
gfx::Size MeasureTextureSize() override;
TextLayoutMode text_layout_mode_ = kMultiLineFixedWidth;
......
......@@ -31,6 +31,9 @@ bool TexturedElement::PrepareToDraw() {
return false;
texture_size_ = MeasureTextureSize();
if (TextureDependsOnMeasurement())
DCHECK(GetTexture()->measured());
return true;
}
......@@ -38,7 +41,12 @@ bool TexturedElement::UpdateTexture() {
if (!GetTexture()->dirty() || !IsVisible())
return false;
// If GL isn't ready yet, or we're in unit tests, present we drew a texture to
// Elements can be dirtied by user input. If the texture draw depends on
// measurement, defer until the next frame.
if (TextureDependsOnMeasurement() && !GetTexture()->measured())
return false;
// If GL isn't ready yet, or we're in unit tests, pretend we drew a texture to
// accurate articulate that we would have. When GL is initialized, all
// textures are dirtied again.
if (!initialized_) {
......
......@@ -44,6 +44,11 @@ class TexturedElement : public UiElement {
bool PrepareToDraw() final;
private:
// Subclasses must return true if redrawing a texture depends on measurement
// (text, for example). If true, a texture dirtied by user input (after
// measurement) will not be redrawn until the following frame.
virtual bool TextureDependsOnMeasurement() const = 0;
virtual gfx::Size MeasureTextureSize() = 0;
gfx::Size texture_size_;
......
......@@ -43,6 +43,7 @@ class UiTexture {
virtual bool LocalHitTest(const gfx::PointF& point) const;
bool measured() const { return measured_; }
bool dirty() const { return dirty_; }
void OnInitialized();
......@@ -136,13 +137,19 @@ class UiTexture {
static void SetForceFontFallbackFailureForTesting(bool force);
void set_dirty() {
measured_ = false;
dirty_ = true;
}
// Textures that depend on measurement to draw must call this when they
// complete measurement work.
void set_measured() { measured_ = true; }
SkColor foreground_color() const;
SkColor background_color() const;
private:
bool measured_ = false;
bool dirty_ = true;
base::Optional<SkColor> foreground_color_;
base::Optional<SkColor> background_color_;
......
......@@ -74,6 +74,10 @@ UiTexture* VectorIcon::GetTexture() const {
return texture_.get();
}
bool VectorIcon::TextureDependsOnMeasurement() const {
return false;
}
gfx::Size VectorIcon::MeasureTextureSize() {
return gfx::Size(texture_width_, texture_width_);
}
......
......@@ -40,6 +40,7 @@ class VectorIcon : public TexturedElement {
UiTexture* GetTexture() const override;
private:
bool TextureDependsOnMeasurement() const override;
gfx::Size MeasureTextureSize() override;
std::unique_ptr<VectorIconTexture> texture_;
......
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