From 736d59c56754b86135d9156208645b8b1814fba1 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Tue, 3 Jan 2017 10:15:33 -0800 Subject: [PATCH] recovery: Fix the broken UI text. UI text is broken (doesn't show any text during FDR) due to commit d530449e54bd327e9c26209ffa0490c6508afe6c, which reordered the calls to RecoveryUI::SetLocale() and RecoveryUI::Init(). Because Init() uses the locale info to load the localized texts (from images), the locale must be set prior to that via SetLocale(). This CL refactors Init() to take the locale parameter, and removes the odd SetLocale() API. Bug: 34029338 Test: 'Run graphics test' under recovery. Change-Id: I620394a3d4e3705e9af5a1f6299285d143ae1b01 --- recovery.cpp | 3 +- screen_ui.cpp | 228 +++++++++++++----------------- screen_ui.h | 8 +- stub_ui.h | 2 - tests/component/verifier_test.cpp | 64 +++++---- ui.cpp | 67 ++++++--- ui.h | 21 ++- wear_ui.cpp | 71 +++++----- wear_ui.h | 4 +- 9 files changed, 238 insertions(+), 230 deletions(-) diff --git a/recovery.cpp b/recovery.cpp index b7aeaee1..5888c542 100644 --- a/recovery.cpp +++ b/recovery.cpp @@ -1506,11 +1506,10 @@ int main(int argc, char **argv) { Device* device = make_device(); ui = device->GetUI(); - if (!ui->Init()) { + if (!ui->Init(locale)) { printf("Failed to initialize UI, use stub UI instead."); ui = new StubRecoveryUI(); } - ui->SetLocale(locale.c_str()); // Set background string to "installing security update" for security update, // otherwise set it to "installing system update". ui->SetSystemUpdateText(security_update); diff --git a/screen_ui.cpp b/screen_ui.cpp index 5b9e5a5a..706877b4 100644 --- a/screen_ui.cpp +++ b/screen_ui.cpp @@ -29,6 +29,7 @@ #include #include +#include #include #include @@ -51,37 +52,34 @@ static double now() { return tv.tv_sec + tv.tv_usec / 1000000.0; } -ScreenRecoveryUI::ScreenRecoveryUI() : - currentIcon(NONE), - locale(nullptr), - progressBarType(EMPTY), - progressScopeStart(0), - progressScopeSize(0), - progress(0), - pagesIdentical(false), - text_cols_(0), - text_rows_(0), - text_(nullptr), - text_col_(0), - text_row_(0), - text_top_(0), - show_text(false), - show_text_ever(false), - menu_(nullptr), - show_menu(false), - menu_items(0), - menu_sel(0), - file_viewer_text_(nullptr), - intro_frames(0), - loop_frames(0), - current_frame(0), - intro_done(false), - animation_fps(30), // TODO: there's currently no way to infer this. - stage(-1), - max_stage(-1), - updateMutex(PTHREAD_MUTEX_INITIALIZER), - rtl_locale(false) { -} +ScreenRecoveryUI::ScreenRecoveryUI() + : currentIcon(NONE), + progressBarType(EMPTY), + progressScopeStart(0), + progressScopeSize(0), + progress(0), + pagesIdentical(false), + text_cols_(0), + text_rows_(0), + text_(nullptr), + text_col_(0), + text_row_(0), + text_top_(0), + show_text(false), + show_text_ever(false), + menu_(nullptr), + show_menu(false), + menu_items(0), + menu_sel(0), + file_viewer_text_(nullptr), + intro_frames(0), + loop_frames(0), + current_frame(0), + intro_done(false), + animation_fps(30), // TODO: there's currently no way to infer this. + stage(-1), + max_stage(-1), + updateMutex(PTHREAD_MUTEX_INITIALIZER) {} GRSurface* ScreenRecoveryUI::GetCurrentFrame() { if (currentIcon == INSTALLING_UPDATE || currentIcon == ERASING) { @@ -175,51 +173,50 @@ void ScreenRecoveryUI::draw_background_locked() { // Does not flip pages. // Should only be called with updateMutex locked. void ScreenRecoveryUI::draw_foreground_locked() { - if (currentIcon != NONE) { - GRSurface* frame = GetCurrentFrame(); - int frame_width = gr_get_width(frame); - int frame_height = gr_get_height(frame); - int frame_x = (gr_fb_width() - frame_width) / 2; - int frame_y = GetAnimationBaseline(); - gr_blit(frame, 0, 0, frame_width, frame_height, frame_x, frame_y); - } + if (currentIcon != NONE) { + GRSurface* frame = GetCurrentFrame(); + int frame_width = gr_get_width(frame); + int frame_height = gr_get_height(frame); + int frame_x = (gr_fb_width() - frame_width) / 2; + int frame_y = GetAnimationBaseline(); + gr_blit(frame, 0, 0, frame_width, frame_height, frame_x, frame_y); + } - if (progressBarType != EMPTY) { - int width = gr_get_width(progressBarEmpty); - int height = gr_get_height(progressBarEmpty); + if (progressBarType != EMPTY) { + int width = gr_get_width(progressBarEmpty); + int height = gr_get_height(progressBarEmpty); - int progress_x = (gr_fb_width() - width)/2; - int progress_y = GetProgressBaseline(); + int progress_x = (gr_fb_width() - width) / 2; + int progress_y = GetProgressBaseline(); - // Erase behind the progress bar (in case this was a progress-only update) - gr_color(0, 0, 0, 255); - gr_fill(progress_x, progress_y, width, height); + // Erase behind the progress bar (in case this was a progress-only update) + gr_color(0, 0, 0, 255); + gr_fill(progress_x, progress_y, width, height); - if (progressBarType == DETERMINATE) { - float p = progressScopeStart + progress * progressScopeSize; - int pos = (int) (p * width); + if (progressBarType == DETERMINATE) { + float p = progressScopeStart + progress * progressScopeSize; + int pos = static_cast(p * width); - if (rtl_locale) { - // Fill the progress bar from right to left. - if (pos > 0) { - gr_blit(progressBarFill, width-pos, 0, pos, height, - progress_x+width-pos, progress_y); - } - if (pos < width-1) { - gr_blit(progressBarEmpty, 0, 0, width-pos, height, progress_x, progress_y); - } - } else { - // Fill the progress bar from left to right. - if (pos > 0) { - gr_blit(progressBarFill, 0, 0, pos, height, progress_x, progress_y); - } - if (pos < width-1) { - gr_blit(progressBarEmpty, pos, 0, width-pos, height, - progress_x+pos, progress_y); - } - } + if (rtl_locale_) { + // Fill the progress bar from right to left. + if (pos > 0) { + gr_blit(progressBarFill, width - pos, 0, pos, height, progress_x + width - pos, + progress_y); } + if (pos < width - 1) { + gr_blit(progressBarEmpty, 0, 0, width - pos, height, progress_x, progress_y); + } + } else { + // Fill the progress bar from left to right. + if (pos > 0) { + gr_blit(progressBarFill, 0, 0, pos, height, progress_x, progress_y); + } + if (pos < width - 1) { + gr_blit(progressBarEmpty, pos, 0, width - pos, height, progress_x + pos, progress_y); + } + } } + } } void ScreenRecoveryUI::SetColor(UIElement e) { @@ -423,10 +420,10 @@ void ScreenRecoveryUI::LoadBitmap(const char* filename, GRSurface** surface) { } void ScreenRecoveryUI::LoadLocalizedBitmap(const char* filename, GRSurface** surface) { - int result = res_create_localized_alpha_surface(filename, locale, surface); - if (result < 0) { - LOG(ERROR) << "couldn't load bitmap " << filename << " (error " << result << ")"; - } + int result = res_create_localized_alpha_surface(filename, locale_.c_str(), surface); + if (result < 0) { + LOG(ERROR) << "couldn't load bitmap " << filename << " (error " << result << ")"; + } } static char** Alloc2d(size_t rows, size_t cols) { @@ -459,47 +456,47 @@ bool ScreenRecoveryUI::InitTextParams() { return true; } -bool ScreenRecoveryUI::Init() { - RecoveryUI::Init(); - if (!InitTextParams()) { - return false; - } +bool ScreenRecoveryUI::Init(const std::string& locale) { + RecoveryUI::Init(locale); + if (!InitTextParams()) { + return false; + } - density_ = static_cast(android::base::GetIntProperty("ro.sf.lcd_density", 160)) / 160.f; + density_ = static_cast(android::base::GetIntProperty("ro.sf.lcd_density", 160)) / 160.f; - // Are we portrait or landscape? - layout_ = (gr_fb_width() > gr_fb_height()) ? LANDSCAPE : PORTRAIT; - // Are we the large variant of our base layout? - if (gr_fb_height() > PixelsFromDp(800)) ++layout_; + // Are we portrait or landscape? + layout_ = (gr_fb_width() > gr_fb_height()) ? LANDSCAPE : PORTRAIT; + // Are we the large variant of our base layout? + if (gr_fb_height() > PixelsFromDp(800)) ++layout_; - text_ = Alloc2d(text_rows_, text_cols_ + 1); - file_viewer_text_ = Alloc2d(text_rows_, text_cols_ + 1); - menu_ = Alloc2d(text_rows_, text_cols_ + 1); + text_ = Alloc2d(text_rows_, text_cols_ + 1); + file_viewer_text_ = Alloc2d(text_rows_, text_cols_ + 1); + menu_ = Alloc2d(text_rows_, text_cols_ + 1); - text_col_ = text_row_ = 0; - text_top_ = 1; + text_col_ = text_row_ = 0; + text_top_ = 1; - LoadBitmap("icon_error", &error_icon); + LoadBitmap("icon_error", &error_icon); - LoadBitmap("progress_empty", &progressBarEmpty); - LoadBitmap("progress_fill", &progressBarFill); + LoadBitmap("progress_empty", &progressBarEmpty); + LoadBitmap("progress_fill", &progressBarFill); - LoadBitmap("stage_empty", &stageMarkerEmpty); - LoadBitmap("stage_fill", &stageMarkerFill); + LoadBitmap("stage_empty", &stageMarkerEmpty); + LoadBitmap("stage_fill", &stageMarkerFill); - // Background text for "installing_update" could be "installing update" - // or "installing security update". It will be set after UI init according - // to commands in BCB. - installing_text = nullptr; - LoadLocalizedBitmap("erasing_text", &erasing_text); - LoadLocalizedBitmap("no_command_text", &no_command_text); - LoadLocalizedBitmap("error_text", &error_text); + // Background text for "installing_update" could be "installing update" + // or "installing security update". It will be set after UI init according + // to commands in BCB. + installing_text = nullptr; + LoadLocalizedBitmap("erasing_text", &erasing_text); + LoadLocalizedBitmap("no_command_text", &no_command_text); + LoadLocalizedBitmap("error_text", &error_text); - LoadAnimation(); + LoadAnimation(); - pthread_create(&progress_thread_, nullptr, ProgressThreadStartRoutine, this); + pthread_create(&progress_thread_, nullptr, ProgressThreadStartRoutine, this); - return true; + return true; } void ScreenRecoveryUI::LoadAnimation() { @@ -539,31 +536,6 @@ void ScreenRecoveryUI::LoadAnimation() { } } -void ScreenRecoveryUI::SetLocale(const char* new_locale) { - this->locale = new_locale; - this->rtl_locale = false; - - if (locale) { - char* lang = strdup(locale); - for (char* p = lang; *p; ++p) { - if (*p == '_') { - *p = '\0'; - break; - } - } - - // A bit cheesy: keep an explicit list of supported RTL languages. - if (strcmp(lang, "ar") == 0 || // Arabic - strcmp(lang, "fa") == 0 || // Persian (Farsi) - strcmp(lang, "he") == 0 || // Hebrew (new language code) - strcmp(lang, "iw") == 0 || // Hebrew (old language code) - strcmp(lang, "ur") == 0) { // Urdu - rtl_locale = true; - } - free(lang); - } -} - void ScreenRecoveryUI::SetBackground(Icon icon) { pthread_mutex_lock(&updateMutex); diff --git a/screen_ui.h b/screen_ui.h index 38e2f072..3ad64907 100644 --- a/screen_ui.h +++ b/screen_ui.h @@ -20,6 +20,8 @@ #include #include +#include + #include "ui.h" #include "minui/minui.h" @@ -29,8 +31,7 @@ class ScreenRecoveryUI : public RecoveryUI { public: ScreenRecoveryUI(); - bool Init() override; - void SetLocale(const char* locale); + bool Init(const std::string& locale) override; // overall recovery state ("background image") void SetBackground(Icon icon); @@ -71,8 +72,6 @@ class ScreenRecoveryUI : public RecoveryUI { protected: Icon currentIcon; - const char* locale; - // The scale factor from dp to pixels. 1.0 for mdpi, 4.0 for xxxhdpi. float density_; // The layout to use. @@ -135,7 +134,6 @@ class ScreenRecoveryUI : public RecoveryUI { int char_width_; int char_height_; pthread_mutex_t updateMutex; - bool rtl_locale; virtual bool InitTextParams(); diff --git a/stub_ui.h b/stub_ui.h index 1219b284..85dbcfd8 100644 --- a/stub_ui.h +++ b/stub_ui.h @@ -24,8 +24,6 @@ class StubRecoveryUI : public RecoveryUI { public: StubRecoveryUI() = default; - void SetLocale(const char* locale) override {} - void SetBackground(Icon icon) override {} void SetSystemUpdateText(bool security_update) override {} diff --git a/tests/component/verifier_test.cpp b/tests/component/verifier_test.cpp index 33aadb3f..b740af96 100644 --- a/tests/component/verifier_test.cpp +++ b/tests/component/verifier_test.cpp @@ -40,38 +40,44 @@ RecoveryUI* ui = NULL; class MockUI : public RecoveryUI { - bool Init() { return true; } - void SetStage(int, int) { } - void SetLocale(const char*) { } - void SetBackground(Icon /*icon*/) { } - void SetSystemUpdateText(bool /*security_update*/) { } + bool Init(const std::string&) override { + return true; + } + void SetStage(int, int) override {} + void SetBackground(Icon /*icon*/) override {} + void SetSystemUpdateText(bool /*security_update*/) override {} - void SetProgressType(ProgressType /*determinate*/) { } - void ShowProgress(float /*portion*/, float /*seconds*/) { } - void SetProgress(float /*fraction*/) { } + void SetProgressType(ProgressType /*determinate*/) override {} + void ShowProgress(float /*portion*/, float /*seconds*/) override {} + void SetProgress(float /*fraction*/) override {} - void ShowText(bool /*visible*/) { } - bool IsTextVisible() { return false; } - bool WasTextEverVisible() { return false; } - void Print(const char* fmt, ...) { - va_list ap; - va_start(ap, fmt); - vfprintf(stderr, fmt, ap); - va_end(ap); - } - void PrintOnScreenOnly(const char* fmt, ...) { - va_list ap; - va_start(ap, fmt); - vfprintf(stderr, fmt, ap); - va_end(ap); - } - void ShowFile(const char*) { } + void ShowText(bool /*visible*/) override {} + bool IsTextVisible() override { + return false; + } + bool WasTextEverVisible() override { + return false; + } + void Print(const char* fmt, ...) override { + va_list ap; + va_start(ap, fmt); + vfprintf(stderr, fmt, ap); + va_end(ap); + } + void PrintOnScreenOnly(const char* fmt, ...) override { + va_list ap; + va_start(ap, fmt); + vfprintf(stderr, fmt, ap); + va_end(ap); + } + void ShowFile(const char*) override {} - void StartMenu(const char* const* /*headers*/, - const char* const* /*items*/, - int /*initial_selection*/) { } - int SelectMenu(int /*sel*/) { return 0; } - void EndMenu() { } + void StartMenu(const char* const* /*headers*/, const char* const* /*items*/, + int /*initial_selection*/) override {} + int SelectMenu(int /*sel*/) override { + return 0; + } + void EndMenu() override {} }; void diff --git a/ui.cpp b/ui.cpp index 2d80c382..f31660db 100644 --- a/ui.cpp +++ b/ui.cpp @@ -14,6 +14,8 @@ * limitations under the License. */ +#include "ui.h" + #include #include #include @@ -28,6 +30,8 @@ #include #include +#include + #include #include @@ -35,25 +39,25 @@ #include "roots.h" #include "device.h" #include "minui/minui.h" -#include "screen_ui.h" -#include "ui.h" #define UI_WAIT_KEY_TIMEOUT_SEC 120 RecoveryUI::RecoveryUI() - : key_queue_len(0), - key_last_down(-1), - key_long_press(false), - key_down_count(0), - enable_reboot(true), - consecutive_power_keys(0), - last_key(-1), - has_power_key(false), - has_up_key(false), - has_down_key(false) { - pthread_mutex_init(&key_queue_mutex, nullptr); - pthread_cond_init(&key_queue_cond, nullptr); - memset(key_pressed, 0, sizeof(key_pressed)); + : locale_(""), + rtl_locale_(false), + key_queue_len(0), + key_last_down(-1), + key_long_press(false), + key_down_count(0), + enable_reboot(true), + consecutive_power_keys(0), + last_key(-1), + has_power_key(false), + has_up_key(false), + has_down_key(false) { + pthread_mutex_init(&key_queue_mutex, nullptr); + pthread_cond_init(&key_queue_cond, nullptr); + memset(key_pressed, 0, sizeof(key_pressed)); } void RecoveryUI::OnKeyDetected(int key_code) { @@ -80,13 +84,16 @@ static void* InputThreadLoop(void*) { return nullptr; } -bool RecoveryUI::Init() { - ev_init(InputCallback, this); +bool RecoveryUI::Init(const std::string& locale) { + // Set up the locale info. + SetLocale(locale); - ev_iterate_available_keys(std::bind(&RecoveryUI::OnKeyDetected, this, std::placeholders::_1)); + ev_init(InputCallback, this); - pthread_create(&input_thread_, nullptr, InputThreadLoop, nullptr); - return true; + ev_iterate_available_keys(std::bind(&RecoveryUI::OnKeyDetected, this, std::placeholders::_1)); + + pthread_create(&input_thread_, nullptr, InputThreadLoop, nullptr); + return true; } int RecoveryUI::OnInputEvent(int fd, uint32_t epevents) { @@ -338,3 +345,23 @@ void RecoveryUI::SetEnableReboot(bool enabled) { enable_reboot = enabled; pthread_mutex_unlock(&key_queue_mutex); } + +void RecoveryUI::SetLocale(const std::string& new_locale) { + this->locale_ = new_locale; + this->rtl_locale_ = false; + + if (!new_locale.empty()) { + size_t underscore = new_locale.find('_'); + // lang has the language prefix prior to '_', or full string if '_' doesn't exist. + std::string lang = new_locale.substr(0, underscore); + + // A bit cheesy: keep an explicit list of supported RTL languages. + if (lang == "ar" || // Arabic + lang == "fa" || // Persian (Farsi) + lang == "he" || // Hebrew (new language code) + lang == "iw" || // Hebrew (old language code) + lang == "ur") { // Urdu + rtl_locale_ = true; + } + } +} diff --git a/ui.h b/ui.h index be95a4e2..8493c6f0 100644 --- a/ui.h +++ b/ui.h @@ -21,6 +21,8 @@ #include #include +#include + // Abstract class for controlling the user interface during recovery. class RecoveryUI { public: @@ -28,14 +30,13 @@ class RecoveryUI { virtual ~RecoveryUI() { } - // Initialize the object; called before anything else. Returns true on success. - virtual bool Init(); + // Initialize the object; called before anything else. UI texts will be + // initialized according to the given locale. Returns true on success. + virtual bool Init(const std::string& locale); + // Show a stage indicator. Call immediately after Init(). virtual void SetStage(int current, int max) = 0; - // After calling Init(), you can tell the UI what locale it is operating in. - virtual void SetLocale(const char* locale) = 0; - // Set the overall recovery state ("background image"). enum Icon { NONE, INSTALLING_UPDATE, ERASING, NO_COMMAND, ERROR }; virtual void SetBackground(Icon icon) = 0; @@ -122,10 +123,14 @@ class RecoveryUI { // statements will be displayed. virtual void EndMenu() = 0; -protected: + protected: void EnqueueKey(int key_code); -private: + // The locale that's used to show the rendered texts. + std::string locale_; + bool rtl_locale_; + + private: // Key event input queue pthread_mutex_t key_queue_mutex; pthread_cond_t key_queue_cond; @@ -162,6 +167,8 @@ private: static void* time_key_helper(void* cookie); void time_key(int key_code, int count); + + void SetLocale(const std::string&); }; #endif // RECOVERY_UI_H diff --git a/wear_ui.cpp b/wear_ui.cpp index bdb0ef00..b4c63a5a 100644 --- a/wear_ui.cpp +++ b/wear_ui.cpp @@ -14,6 +14,8 @@ * limitations under the License. */ +#include "wear_ui.h" + #include #include #include @@ -25,11 +27,11 @@ #include #include +#include #include #include "common.h" #include "device.h" -#include "wear_ui.h" #include "android-base/properties.h" #include "android-base/strings.h" #include "android-base/stringprintf.h" @@ -204,51 +206,48 @@ bool WearRecoveryUI::InitTextParams() { return true; } -bool WearRecoveryUI::Init() { - if (!ScreenRecoveryUI::Init()) { - return false; - } +bool WearRecoveryUI::Init(const std::string& locale) { + if (!ScreenRecoveryUI::Init(locale)) { + return false; + } - LoadBitmap("icon_error", &backgroundIcon[ERROR]); - backgroundIcon[NO_COMMAND] = backgroundIcon[ERROR]; + LoadBitmap("icon_error", &backgroundIcon[ERROR]); + backgroundIcon[NO_COMMAND] = backgroundIcon[ERROR]; - // This leaves backgroundIcon[INSTALLING_UPDATE] and backgroundIcon[ERASING] - // as NULL which is fine since draw_background_locked() doesn't use them. + // This leaves backgroundIcon[INSTALLING_UPDATE] and backgroundIcon[ERASING] + // as NULL which is fine since draw_background_locked() doesn't use them. - return true; + return true; } -void WearRecoveryUI::SetStage(int current, int max) -{ -} +void WearRecoveryUI::SetStage(int current, int max) {} -void WearRecoveryUI::Print(const char *fmt, ...) -{ - char buf[256]; - va_list ap; - va_start(ap, fmt); - vsnprintf(buf, 256, fmt, ap); - va_end(ap); +void WearRecoveryUI::Print(const char* fmt, ...) { + char buf[256]; + va_list ap; + va_start(ap, fmt); + vsnprintf(buf, 256, fmt, ap); + va_end(ap); - fputs(buf, stdout); + fputs(buf, stdout); - // This can get called before ui_init(), so be careful. - pthread_mutex_lock(&updateMutex); - if (text_rows_ > 0 && text_cols_ > 0) { - char *ptr; - for (ptr = buf; *ptr != '\0'; ++ptr) { - if (*ptr == '\n' || text_col_ >= text_cols_) { - text_[text_row_][text_col_] = '\0'; - text_col_ = 0; - text_row_ = (text_row_ + 1) % text_rows_; - if (text_row_ == text_top_) text_top_ = (text_top_ + 1) % text_rows_; - } - if (*ptr != '\n') text_[text_row_][text_col_++] = *ptr; - } + // This can get called before ui_init(), so be careful. + pthread_mutex_lock(&updateMutex); + if (text_rows_ > 0 && text_cols_ > 0) { + char* ptr; + for (ptr = buf; *ptr != '\0'; ++ptr) { + if (*ptr == '\n' || text_col_ >= text_cols_) { text_[text_row_][text_col_] = '\0'; - update_screen_locked(); + text_col_ = 0; + text_row_ = (text_row_ + 1) % text_rows_; + if (text_row_ == text_top_) text_top_ = (text_top_ + 1) % text_rows_; + } + if (*ptr != '\n') text_[text_row_][text_col_++] = *ptr; } - pthread_mutex_unlock(&updateMutex); + text_[text_row_][text_col_] = '\0'; + update_screen_locked(); + } + pthread_mutex_unlock(&updateMutex); } void WearRecoveryUI::StartMenu(const char* const * headers, const char* const * items, diff --git a/wear_ui.h b/wear_ui.h index 5ac6f49d..4cd852f2 100644 --- a/wear_ui.h +++ b/wear_ui.h @@ -19,11 +19,13 @@ #include "screen_ui.h" +#include + class WearRecoveryUI : public ScreenRecoveryUI { public: WearRecoveryUI(); - bool Init() override; + bool Init(const std::string& locale) override; void SetStage(int current, int max) override;