From 44820ac1e31ffa029ab5baa71238a11b6db3e6cc Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Tue, 30 Oct 2018 23:34:50 -0700 Subject: minui: Add a protected GRSurface ctor. This prepares for the removal of the default and copy ctors, by making GRSurface::Create() as the only way to get GRSurface instances. Test: mmma -j bootable/recovery Test: Run recovery_unit_test on marlin. Change-Id: I0c34c3f3967e252deb020907c83acbac8a8f36b9 --- minui/include/minui/minui.h | 13 ++-- minui/resources.cpp | 137 +++++++++++++++++------------------------- tests/unit/minui_test.cpp | 2 +- tests/unit/screen_ui_test.cpp | 41 ++++++------- 4 files changed, 86 insertions(+), 107 deletions(-) diff --git a/minui/include/minui/minui.h b/minui/include/minui/minui.h index e9bd1c4f1..66d992b93 100644 --- a/minui/include/minui/minui.h +++ b/minui/include/minui/minui.h @@ -33,10 +33,11 @@ class GRSurface { GRSurface() = default; virtual ~GRSurface(); - // Creates and returns a GRSurface instance for the given data_size. The starting address of the - // surface data is aligned to SURFACE_DATA_ALIGNMENT. Returns the created GRSurface instance (in - // std::unique_ptr), or nullptr on error. - static std::unique_ptr Create(size_t data_size); + // Creates and returns a GRSurface instance that's sufficient for storing an image of the given + // size. The starting address of the surface data is aligned to SURFACE_DATA_ALIGNMENT. Returns + // the created GRSurface instance (in std::unique_ptr), or nullptr on error. + static std::unique_ptr Create(int width, int height, int row_bytes, int pixel_bytes, + size_t data_size); virtual uint8_t* data() { return data_; @@ -51,6 +52,10 @@ class GRSurface { int row_bytes; int pixel_bytes; + protected: + GRSurface(int width, int height, int row_bytes, int pixel_bytes) + : width(width), height(height), row_bytes(row_bytes), pixel_bytes(pixel_bytes) {} + private: uint8_t* data_{ nullptr }; }; diff --git a/minui/resources.cpp b/minui/resources.cpp index c01c1868e..9c9af0242 100644 --- a/minui/resources.cpp +++ b/minui/resources.cpp @@ -39,9 +39,11 @@ static std::string g_resource_dir{ "/res/images" }; -std::unique_ptr GRSurface::Create(size_t data_size) { +std::unique_ptr GRSurface::Create(int width, int height, int row_bytes, int pixel_bytes, + size_t data_size) { static constexpr size_t kSurfaceDataAlignment = 8; - std::unique_ptr result = std::make_unique(); + // Cannot use std::make_unique to access non-public ctor. + auto result = std::unique_ptr(new GRSurface(width, height, row_bytes, pixel_bytes)); size_t aligned_size = (data_size + kSurfaceDataAlignment - 1) / kSurfaceDataAlignment * kSurfaceDataAlignment; result->data_ = static_cast(aligned_alloc(kSurfaceDataAlignment, aligned_size)); @@ -68,7 +70,7 @@ PngHandler::PngHandler(const std::string& name) { return; } - unsigned char header[8]; + uint8_t header[8]; size_t bytesRead = fread(header, 1, sizeof(header), png_fp_.get()); if (bytesRead != sizeof(header)) { error_code_ = -2; @@ -131,70 +133,49 @@ PngHandler::~PngHandler() { } } -// "display" surfaces are transformed into the framebuffer's required -// pixel format (currently only RGBX is supported) at load time, so -// gr_blit() can be nothing more than a memcpy() for each row. The -// next two functions are the only ones that know anything about the -// framebuffer pixel format; they need to be modified if the -// framebuffer format changes (but nothing else should). - -// Allocates and returns a GRSurface* sufficient for storing an image of the indicated size in the -// framebuffer pixel format. -static std::unique_ptr init_display_surface(png_uint_32 width, png_uint_32 height) { - std::unique_ptr surface = GRSurface::Create(width * height * 4); - if (!surface) return nullptr; - - surface->width = width; - surface->height = height; - surface->row_bytes = width * 4; - surface->pixel_bytes = 4; - - return surface; -} +// "display" surfaces are transformed into the framebuffer's required pixel format (currently only +// RGBX is supported) at load time, so gr_blit() can be nothing more than a memcpy() for each row. -// Copy 'input_row' to 'output_row', transforming it to the -// framebuffer pixel format. The input format depends on the value of -// 'channels': +// Copies 'input_row' to 'output_row', transforming it to the framebuffer pixel format. The input +// format depends on the value of 'channels': // // 1 - input is 8-bit grayscale // 3 - input is 24-bit RGB // 4 - input is 32-bit RGBA/RGBX // // 'width' is the number of pixels in the row. -static void transform_rgb_to_draw(unsigned char* input_row, - unsigned char* output_row, - int channels, int width) { - int x; - unsigned char* ip = input_row; - unsigned char* op = output_row; - - switch (channels) { - case 1: - // expand gray level to RGBX - for (x = 0; x < width; ++x) { - *op++ = *ip; - *op++ = *ip; - *op++ = *ip; - *op++ = 0xff; - ip++; - } - break; - - case 3: - // expand RGBA to RGBX - for (x = 0; x < width; ++x) { - *op++ = *ip++; - *op++ = *ip++; - *op++ = *ip++; - *op++ = 0xff; - } - break; - - case 4: - // copy RGBA to RGBX - memcpy(output_row, input_row, width*4); - break; - } +static void TransformRgbToDraw(const uint8_t* input_row, uint8_t* output_row, int channels, + int width) { + const uint8_t* ip = input_row; + uint8_t* op = output_row; + + switch (channels) { + case 1: + // expand gray level to RGBX + for (int x = 0; x < width; ++x) { + *op++ = *ip; + *op++ = *ip; + *op++ = *ip; + *op++ = 0xff; + ip++; + } + break; + + case 3: + // expand RGBA to RGBX + for (int x = 0; x < width; ++x) { + *op++ = *ip++; + *op++ = *ip++; + *op++ = *ip++; + *op++ = 0xff; + } + break; + + case 4: + // copy RGBA to RGBX + memcpy(output_row, input_row, width * 4); + break; + } } int res_create_display_surface(const char* name, GRSurface** pSurface) { @@ -207,7 +188,7 @@ int res_create_display_surface(const char* name, GRSurface** pSurface) { png_uint_32 width = png_handler.width(); png_uint_32 height = png_handler.height(); - std::unique_ptr surface = init_display_surface(width, height); + auto surface = GRSurface::Create(width, height, width * 4, 4, width * height * 4); if (!surface) { return -8; } @@ -218,10 +199,10 @@ int res_create_display_surface(const char* name, GRSurface** pSurface) { } for (png_uint_32 y = 0; y < height; ++y) { - std::vector p_row(width * 4); + std::vector p_row(width * 4); png_read_row(png_ptr, p_row.data(), nullptr); - transform_rgb_to_draw(p_row.data(), surface->data() + y * surface->row_bytes, - png_handler.channels(), width); + TransformRgbToDraw(p_row.data(), surface->data() + y * surface->row_bytes, + png_handler.channels(), width); } *pSurface = surface.release(); @@ -277,7 +258,9 @@ int res_create_multi_display_surface(const char* name, int* frames, int* fps, goto exit; } for (int i = 0; i < *frames; ++i) { - auto created_surface = init_display_surface(width, height / *frames); + auto height_per_frame = height / *frames; + auto created_surface = + GRSurface::Create(width, height_per_frame, width * 4, 4, width * height_per_frame); if (!created_surface) { result = -8; goto exit; @@ -290,11 +273,11 @@ int res_create_multi_display_surface(const char* name, int* frames, int* fps, } for (png_uint_32 y = 0; y < height; ++y) { - std::vector p_row(width * 4); + std::vector p_row(width * 4); png_read_row(png_ptr, p_row.data(), nullptr); int frame = y % *frames; - unsigned char* out_row = surface[frame]->data() + (y / *frames) * surface[frame]->row_bytes; - transform_rgb_to_draw(p_row.data(), out_row, png_handler.channels(), width); + uint8_t* out_row = surface[frame]->data() + (y / *frames) * surface[frame]->row_bytes; + TransformRgbToDraw(p_row.data(), out_row, png_handler.channels(), width); } *pSurface = surface; @@ -325,14 +308,10 @@ int res_create_alpha_surface(const char* name, GRSurface** pSurface) { png_uint_32 width = png_handler.width(); png_uint_32 height = png_handler.height(); - std::unique_ptr surface = GRSurface::Create(width * height); + auto surface = GRSurface::Create(width, height, width, 1, width * height); if (!surface) { return -8; } - surface->width = width; - surface->height = height; - surface->row_bytes = width; - surface->pixel_bytes = 1; PixelFormat pixel_format = gr_pixel_format(); if (pixel_format == PixelFormat::ABGR || pixel_format == PixelFormat::BGRA) { @@ -340,7 +319,7 @@ int res_create_alpha_surface(const char* name, GRSurface** pSurface) { } for (png_uint_32 y = 0; y < height; ++y) { - unsigned char* p_row = surface->data() + y * surface->row_bytes; + uint8_t* p_row = surface->data() + y * surface->row_bytes; png_read_row(png_ptr, p_row, nullptr); } @@ -389,7 +368,7 @@ std::vector get_locales_in_png(const std::string& png_name) { } std::vector result; - std::vector row(png_handler.width()); + std::vector row(png_handler.width()); for (png_uint_32 y = 0; y < png_handler.height(); ++y) { png_read_row(png_handler.png_ptr(), row.data(), nullptr); int h = (row[3] << 8) | row[2]; @@ -425,7 +404,7 @@ int res_create_localized_alpha_surface(const char* name, png_uint_32 height = png_handler.height(); for (png_uint_32 y = 0; y < height; ++y) { - std::vector row(width); + std::vector row(width); png_read_row(png_ptr, row.data(), nullptr); int w = (row[1] << 8) | row[0]; int h = (row[3] << 8) | row[2]; @@ -435,14 +414,10 @@ int res_create_localized_alpha_surface(const char* name, if (y + 1 + h >= height || matches_locale(loc, locale)) { printf(" %20s: %s (%d x %d @ %d)\n", name, loc, w, h, y); - std::unique_ptr surface = GRSurface::Create(w * h); + auto surface = GRSurface::Create(w, h, w, 1, w * h); if (!surface) { return -8; } - surface->width = w; - surface->height = h; - surface->row_bytes = w; - surface->pixel_bytes = 1; for (int i = 0; i < h; ++i, ++y) { png_read_row(png_ptr, row.data(), nullptr); diff --git a/tests/unit/minui_test.cpp b/tests/unit/minui_test.cpp index cad6a3d79..b188b5992 100644 --- a/tests/unit/minui_test.cpp +++ b/tests/unit/minui_test.cpp @@ -25,7 +25,7 @@ TEST(GRSurfaceTest, Create_aligned) { static constexpr size_t kSurfaceDataAlignment = 8; for (size_t data_size = 100; data_size < 128; data_size++) { - std::unique_ptr surface(GRSurface::Create(data_size)); + auto surface = GRSurface::Create(10, 1, 10, 1, data_size); ASSERT_TRUE(surface); ASSERT_EQ(0, reinterpret_cast(surface->data()) % kSurfaceDataAlignment); } diff --git a/tests/unit/screen_ui_test.cpp b/tests/unit/screen_ui_test.cpp index 0014e45f1..2b76944b1 100644 --- a/tests/unit/screen_ui_test.cpp +++ b/tests/unit/screen_ui_test.cpp @@ -69,17 +69,6 @@ class ScreenUITest : public testing::Test { MockDrawFunctions draw_funcs_; }; -// TODO(xunchang) Create a constructor. -static GRSurface CreateFakeGRSurface(int width, int height, int row_bytes, int pixel_bytes) { - GRSurface fake_surface; - fake_surface.width = width; - fake_surface.height = height; - fake_surface.row_bytes = row_bytes; - fake_surface.pixel_bytes = pixel_bytes; - - return fake_surface; -} - TEST_F(ScreenUITest, StartPhoneMenuSmoke) { TextMenu menu(false, 10, 20, HEADERS, ITEMS, 0, 20, draw_funcs_); ASSERT_FALSE(menu.scrollable()); @@ -241,9 +230,14 @@ TEST_F(ScreenUITest, WearMenuSelectItemsOverflow) { } TEST_F(ScreenUITest, GraphicMenuSelection) { - auto fake_surface = CreateFakeGRSurface(50, 50, 50, 1); - std::vector items = { &fake_surface, &fake_surface, &fake_surface }; - GraphicMenu menu(&fake_surface, items, 0, draw_funcs_); + auto header = GRSurface::Create(50, 50, 50, 1, 50 * 50); + auto item = GRSurface::Create(50, 50, 50, 1, 50 * 50); + std::vector items = { + item.get(), + item.get(), + item.get(), + }; + GraphicMenu menu(header.get(), items, 0, draw_funcs_); ASSERT_EQ(0, menu.selection()); @@ -263,18 +257,23 @@ TEST_F(ScreenUITest, GraphicMenuSelection) { } TEST_F(ScreenUITest, GraphicMenuValidate) { - auto fake_surface = CreateFakeGRSurface(50, 50, 50, 1); - std::vector items = { &fake_surface, &fake_surface, &fake_surface }; + auto header = GRSurface::Create(50, 50, 50, 1, 50 * 50); + auto item = GRSurface::Create(50, 50, 50, 1, 50 * 50); + std::vector items = { + item.get(), + item.get(), + item.get(), + }; - ASSERT_TRUE(GraphicMenu::Validate(200, 200, &fake_surface, items)); + ASSERT_TRUE(GraphicMenu::Validate(200, 200, header.get(), items)); // Menu exceeds the horizontal boundary. - auto wide_surface = CreateFakeGRSurface(300, 50, 300, 1); - ASSERT_FALSE(GraphicMenu::Validate(299, 200, &wide_surface, items)); + auto wide_surface = GRSurface::Create(300, 50, 300, 1, 300 * 50); + ASSERT_FALSE(GraphicMenu::Validate(299, 200, wide_surface.get(), items)); // Menu exceeds the vertical boundary. - items.push_back(&fake_surface); - ASSERT_FALSE(GraphicMenu::Validate(200, 249, &fake_surface, items)); + items.push_back(item.get()); + ASSERT_FALSE(GraphicMenu::Validate(200, 249, header.get(), items)); } static constexpr int kMagicAction = 101; -- cgit v1.2.3