From 3250f723602244cd3a87327a14755dcde2f4e5dc Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Thu, 29 Jun 2017 14:32:05 -0700 Subject: screen_ui: Compute the top and bottom gaps. We're not actually following the gaps as in the comments. For example, Nexus 6P is supposed to use 220dp and 194dp gaps (top and bottom respectively), but the actual numbers are 185dp and 194dp. Because the animation icon and text sizes don't match the ones claimed (animation: expected 200dp or 700px, actual 800px; text: claimed 14sp, actual 76px). The top gap changes (shrinks) as we compute the baselines bottom-up. This CL switches to using computed gaps: the major UI elements always stay vertically centered, with identical top and bottom gaps. Bug: 63093285 Test: 'Run graphics test' on angler/volantis/fugu/ryu. Change-Id: I3cadbb34f728cf034afa47ac02a6deba8cb6b4e7 --- screen_ui.cpp | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/screen_ui.cpp b/screen_ui.cpp index bcfaaa4be..378a4514a 100644 --- a/screen_ui.cpp +++ b/screen_ui.cpp @@ -112,24 +112,24 @@ int ScreenRecoveryUI::PixelsFromDp(int dp) const { // | portrait large landscape large // ---------+------------------------------------------------- -// gap | 220dp 366dp 142dp 284dp +// gap | // icon | (200dp) // gap | 68dp 68dp 56dp 112dp // text | (14sp) // gap | 32dp 32dp 26dp 52dp // progress | (2dp) -// gap | 194dp 340dp 131dp 262dp +// gap | -// Note that "baseline" is actually the *top* of each icon (because that's how our drawing -// routines work), so that's the more useful measurement for calling code. +// Note that "baseline" is actually the *top* of each icon (because that's how our drawing routines +// work), so that's the more useful measurement for calling code. We use even top and bottom gaps. enum Layout { PORTRAIT = 0, PORTRAIT_LARGE = 1, LANDSCAPE = 2, LANDSCAPE_LARGE = 3, LAYOUT_MAX }; -enum Dimension { PROGRESS = 0, TEXT = 1, ICON = 2, DIMENSION_MAX }; +enum Dimension { TEXT = 0, ICON = 1, DIMENSION_MAX }; static constexpr int kLayouts[LAYOUT_MAX][DIMENSION_MAX] = { - { 194, 32, 68, }, // PORTRAIT - { 340, 32, 68, }, // PORTRAIT_LARGE - { 131, 26, 56, }, // LANDSCAPE - { 262, 52, 112, }, // LANDSCAPE_LARGE + { 32, 68, }, // PORTRAIT + { 32, 68, }, // PORTRAIT_LARGE + { 26, 56, }, // LANDSCAPE + { 52, 112, }, // LANDSCAPE_LARGE }; int ScreenRecoveryUI::GetAnimationBaseline() const { @@ -142,8 +142,11 @@ int ScreenRecoveryUI::GetTextBaseline() const { } int ScreenRecoveryUI::GetProgressBaseline() const { - return gr_fb_height() - PixelsFromDp(kLayouts[layout_][PROGRESS]) - - gr_get_height(progressBarFill); + int elements_sum = gr_get_height(loopFrames[0]) + PixelsFromDp(kLayouts[layout_][ICON]) + + gr_get_height(installing_text) + PixelsFromDp(kLayouts[layout_][TEXT]) + + gr_get_height(progressBarFill); + int bottom_gap = (gr_fb_height() - elements_sum) / 2; + return gr_fb_height() - bottom_gap - gr_get_height(progressBarFill); } // Clear the screen and draw the currently selected background icon (if any). -- cgit v1.2.3 From f95e686dd04056db2ff0a6b2933cce07eba54e14 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Thu, 29 Jun 2017 14:32:05 -0700 Subject: screen_ui: Compute the top and bottom gaps. We're not actually following the gaps as in the comments. For example, Nexus 6P is supposed to use 220dp and 194dp gaps (top and bottom respectively), but the actual numbers are 185dp and 194dp. Because the animation icon and text sizes don't match the ones claimed (animation: expected 200dp or 700px, actual 800px; text: claimed 14sp, actual 76px). The top gap changes (shrinks) as we compute the baselines bottom-up. This CL switches to using computed gaps: the major UI elements always stay vertically centered, with identical top and bottom gaps. Bug: 63093285 Test: 'Run graphics test' on angler/volantis/fugu/ryu. Change-Id: I3cadbb34f728cf034afa47ac02a6deba8cb6b4e7 (cherry picked from commit 3250f723602244cd3a87327a14755dcde2f4e5dc) --- screen_ui.cpp | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/screen_ui.cpp b/screen_ui.cpp index 4ca96a9b2..7334b713e 100644 --- a/screen_ui.cpp +++ b/screen_ui.cpp @@ -107,24 +107,24 @@ int ScreenRecoveryUI::PixelsFromDp(int dp) const { // | portrait large landscape large // ---------+------------------------------------------------- -// gap | 220dp 366dp 142dp 284dp +// gap | // icon | (200dp) // gap | 68dp 68dp 56dp 112dp // text | (14sp) // gap | 32dp 32dp 26dp 52dp // progress | (2dp) -// gap | 194dp 340dp 131dp 262dp +// gap | -// Note that "baseline" is actually the *top* of each icon (because that's how our drawing -// routines work), so that's the more useful measurement for calling code. +// Note that "baseline" is actually the *top* of each icon (because that's how our drawing routines +// work), so that's the more useful measurement for calling code. We use even top and bottom gaps. enum Layout { PORTRAIT = 0, PORTRAIT_LARGE = 1, LANDSCAPE = 2, LANDSCAPE_LARGE = 3, LAYOUT_MAX }; -enum Dimension { PROGRESS = 0, TEXT = 1, ICON = 2, DIMENSION_MAX }; +enum Dimension { TEXT = 0, ICON = 1, DIMENSION_MAX }; static constexpr int kLayouts[LAYOUT_MAX][DIMENSION_MAX] = { - { 194, 32, 68, }, // PORTRAIT - { 340, 32, 68, }, // PORTRAIT_LARGE - { 131, 26, 56, }, // LANDSCAPE - { 262, 52, 112, }, // LANDSCAPE_LARGE + { 32, 68, }, // PORTRAIT + { 32, 68, }, // PORTRAIT_LARGE + { 26, 56, }, // LANDSCAPE + { 52, 112, }, // LANDSCAPE_LARGE }; int ScreenRecoveryUI::GetAnimationBaseline() { @@ -138,8 +138,11 @@ int ScreenRecoveryUI::GetTextBaseline() { } int ScreenRecoveryUI::GetProgressBaseline() { - return gr_fb_height() - PixelsFromDp(kLayouts[layout_][PROGRESS]) - - gr_get_height(progressBarFill); + int elements_sum = gr_get_height(loopFrames[0]) + PixelsFromDp(kLayouts[layout_][ICON]) + + gr_get_height(installing_text) + PixelsFromDp(kLayouts[layout_][TEXT]) + + gr_get_height(progressBarFill); + int bottom_gap = (gr_fb_height() - elements_sum) / 2; + return gr_fb_height() - bottom_gap - gr_get_height(progressBarFill); } // Clear the screen and draw the currently selected background icon (if any). -- cgit v1.2.3 From ea78d86b44661c69fcb740b9d40b47af821dbd57 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Wed, 28 Jun 2017 14:52:17 -0700 Subject: Update ScreenRecoveryUI::Draw* function signatures. Move away from taking int* for the Y-offset. Change it to int and return the offset instead. Test: Check the recovery menu and 'Wipe data' menu. Change-Id: Ib15e070a0d576a0f8f66f35605cb8479e7071f26 --- screen_ui.cpp | 45 +++++++++++++++++++++------------------------ screen_ui.h | 10 +++++++--- vr_ui.cpp | 8 ++++---- vr_ui.h | 2 +- wear_ui.cpp | 10 +++++----- 5 files changed, 38 insertions(+), 37 deletions(-) diff --git a/screen_ui.cpp b/screen_ui.cpp index 378a4514a..d9574d869 100644 --- a/screen_ui.cpp +++ b/screen_ui.cpp @@ -177,9 +177,8 @@ void ScreenRecoveryUI::draw_background_locked() { } } -// Draws the animation and progress bar (if any) on the screen. -// Does not flip pages. -// Should only be called with updateMutex locked. +// Draws the animation and progress bar (if any) on the screen. Does not flip pages. Should only be +// called with updateMutex locked. void ScreenRecoveryUI::draw_foreground_locked() { if (currentIcon != NONE) { GRSurface* frame = GetCurrentFrame(); @@ -257,26 +256,26 @@ void ScreenRecoveryUI::SetColor(UIElement e) const { } } -void ScreenRecoveryUI::DrawHorizontalRule(int* y) const { - SetColor(MENU); - *y += 4; - gr_fill(0, *y, gr_fb_width(), *y + 2); - *y += 4; +int ScreenRecoveryUI::DrawHorizontalRule(int y) const { + gr_fill(0, y + 4, gr_fb_width(), y + 6); + return 8; } void ScreenRecoveryUI::DrawHighlightBar(int x, int y, int width, int height) const { gr_fill(x, y, x + width, y + height); } -void ScreenRecoveryUI::DrawTextLine(int x, int* y, const char* line, bool bold) const { - gr_text(gr_sys_font(), x, *y, line, bold); - *y += char_height_ + 4; +int ScreenRecoveryUI::DrawTextLine(int x, int y, const char* line, bool bold) const { + gr_text(gr_sys_font(), x, y, line, bold); + return char_height_ + 4; } -void ScreenRecoveryUI::DrawTextLines(int x, int* y, const char* const* lines) const { +int ScreenRecoveryUI::DrawTextLines(int x, int y, const char* const* lines) const { + int offset = 0; for (size_t i = 0; lines != nullptr && lines[i] != nullptr; ++i) { - DrawTextLine(x, y, lines[i], false); + offset += DrawTextLine(x, y + offset, lines[i], false); } + return offset; } static const char* REGULAR_HELP[] = { @@ -310,18 +309,17 @@ void ScreenRecoveryUI::draw_screen_locked() { android::base::GetProperty("ro.bootimage.build.fingerprint", ""); SetColor(INFO); - DrawTextLine(x, &y, "Android Recovery", true); + y += DrawTextLine(x, y, "Android Recovery", true); for (const auto& chunk : android::base::Split(recovery_fingerprint, ":")) { - DrawTextLine(x, &y, chunk.c_str(), false); + y += DrawTextLine(x, y, chunk.c_str(), false); } - DrawTextLines(x, &y, HasThreeButtons() ? REGULAR_HELP : LONG_PRESS_HELP); + y += DrawTextLines(x, y, HasThreeButtons() ? REGULAR_HELP : LONG_PRESS_HELP); SetColor(HEADER); - DrawTextLines(x, &y, menu_headers_); + y += DrawTextLines(x, y, menu_headers_); SetColor(MENU); - DrawHorizontalRule(&y); - y += 4; + y += DrawHorizontalRule(y) + 4; for (int i = 0; i < menu_items; ++i) { if (i == menu_sel) { // Draw the highlight bar. @@ -329,13 +327,13 @@ void ScreenRecoveryUI::draw_screen_locked() { DrawHighlightBar(0, y - 2, gr_fb_width(), char_height_ + 4); // Bold white text for the selected item. SetColor(MENU_SEL_FG); - DrawTextLine(x, &y, menu_[i], true); + y += DrawTextLine(x, y, menu_[i], true); SetColor(MENU); } else { - DrawTextLine(x, &y, menu_[i], false); + y += DrawTextLine(x, y, menu_[i], false); } } - DrawHorizontalRule(&y); + y += DrawHorizontalRule(y); } // Display from the bottom up, until we hit the top of the screen, the bottom of the menu, or @@ -345,8 +343,7 @@ void ScreenRecoveryUI::draw_screen_locked() { size_t count = 0; for (int ty = gr_fb_height() - kMarginHeight - char_height_; ty >= y && count < text_rows_; ty -= char_height_, ++count) { - int temp_y = ty; - DrawTextLine(x, &temp_y, text_[row], false); + DrawTextLine(x, ty, text_[row], false); --row; if (row < 0) row = text_rows_ - 1; } diff --git a/screen_ui.h b/screen_ui.h index e8d3c3222..8402fac00 100644 --- a/screen_ui.h +++ b/screen_ui.h @@ -179,10 +179,14 @@ class ScreenRecoveryUI : public RecoveryUI { virtual int GetProgressBaseline() const; virtual int GetTextBaseline() const; - virtual void DrawHorizontalRule(int* y) const; + // Draws a highlight bar at (x, y) - (x + width, y + height). virtual void DrawHighlightBar(int x, int y, int width, int height) const; - virtual void DrawTextLine(int x, int* y, const char* line, bool bold) const; - void DrawTextLines(int x, int* y, const char* const* lines) const; + // Draws a horizontal rule at Y. Returns the offset it should be moving along Y-axis. + virtual int DrawHorizontalRule(int y) const; + // Draws a line of text. Returns the offset it should be moving along Y-axis. + virtual int DrawTextLine(int x, int y, const char* line, bool bold) const; + // Draws multiple text lines. Returns the offset it should be moving along Y-axis. + int DrawTextLines(int x, int y, const char* const* lines) const; }; #endif // RECOVERY_UI_H diff --git a/vr_ui.cpp b/vr_ui.cpp index 8b8261e35..125167268 100644 --- a/vr_ui.cpp +++ b/vr_ui.cpp @@ -27,9 +27,9 @@ bool VrRecoveryUI::InitTextParams() { return true; } -void VrRecoveryUI::DrawTextLine(int x, int* y, const char* line, bool bold) const { +int VrRecoveryUI::DrawTextLine(int x, int y, const char* line, bool bold) const { int mid_divide = gr_fb_width() / 2; - gr_text(gr_sys_font(), x + kStereoOffset, *y, line, bold); - gr_text(gr_sys_font(), x - kStereoOffset + mid_divide, *y, line, bold); - *y += char_height_ + 4; + gr_text(gr_sys_font(), x + kStereoOffset, y, line, bold); + gr_text(gr_sys_font(), x - kStereoOffset + mid_divide, y, line, bold); + return char_height_ + 4; } diff --git a/vr_ui.h b/vr_ui.h index da2163402..d996c145f 100644 --- a/vr_ui.h +++ b/vr_ui.h @@ -30,7 +30,7 @@ class VrRecoveryUI : public ScreenRecoveryUI { bool InitTextParams() override; - void DrawTextLine(int x, int* y, const char* line, bool bold) const override; + int DrawTextLine(int x, int y, const char* line, bool bold) const override; }; #endif // RECOVERY_VR_UI_H diff --git a/wear_ui.cpp b/wear_ui.cpp index a29746cdc..18c30d34a 100644 --- a/wear_ui.cpp +++ b/wear_ui.cpp @@ -95,7 +95,7 @@ void WearRecoveryUI::draw_background_locked() { } } -static const char* HEADERS[] = { +static const char* SWIPE_HELP[] = { "Swipe up/down to move.", "Swipe left/right to select.", "", @@ -119,15 +119,15 @@ void WearRecoveryUI::draw_screen_locked() { std::string recovery_fingerprint = android::base::GetProperty("ro.bootimage.build.fingerprint", ""); SetColor(HEADER); - DrawTextLine(x + 4, &y, "Android Recovery", true); + y += DrawTextLine(x + 4, y, "Android Recovery", true); for (auto& chunk : android::base::Split(recovery_fingerprint, ":")) { - DrawTextLine(x + 4, &y, chunk.c_str(), false); + y += DrawTextLine(x + 4, y, chunk.c_str(), false); } // This is actually the help strings. - DrawTextLines(x + 4, &y, HEADERS); + y += DrawTextLines(x + 4, y, SWIPE_HELP); SetColor(HEADER); - DrawTextLines(x + 4, &y, menu_headers_); + y += DrawTextLines(x + 4, y, menu_headers_); // Show the current menu item number in relation to total number if // items don't fit on the screen. -- cgit v1.2.3