Skip to content

Commit

Permalink
Fix infoPanel crashes when loading incompatible images, that also nee…
Browse files Browse the repository at this point in the history
…d to be stretched (#1418)

Closes #1386 

The images that crashed the infoPanel were in a different format, which
SDL_SoftStretch couldn't handle, leaving image_to_draw empty and
subsequently crashing when drawing the empty image.
To safeguard against this, create a separate surface and check the
return of SoftStretch for error.

Testing this showed that some of the stretched images looked horrible,
so now only stretch images if they are too big to fit the screen. (MMv4
752x560 screenshots mainly)

Additionally, add some error handling to the Gallery launch script, so
it would show "Error" instead of "No screenshots found" when infoPanel
crashes to be less misleading.

To test, copy the screenshot folder from the issue #1386 to your
SD/Screenshots and confirm it doesn't crash when loading the different
screenshots.

---------

Co-authored-by: XK <[email protected]>
Co-authored-by: Aemiii91 <[email protected]>
  • Loading branch information
3 people authored Feb 24, 2024
1 parent 9459f80 commit 575a70d
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 56 deletions.
5 changes: 2 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -262,15 +262,14 @@ patch:
@chmod a+x $(ROOT_DIR)/.github/create_patch.sh && $(ROOT_DIR)/.github/create_patch.sh

external-libs:
@cd $(ROOT_DIR)/include/cJSON && make clean && make
@cd $(ROOT_DIR)/include/SDL && make clean && make

test:
test: external-libs
@mkdir -p $(BUILD_TEST_DIR)/infoPanel_test_data && cd $(TEST_SRC_DIR) && BUILD_DIR=$(BUILD_TEST_DIR)/ make dev
@cp -R $(TEST_SRC_DIR)/infoPanel_test_data $(BUILD_TEST_DIR)/
cd $(BUILD_TEST_DIR) && ./test

static-analysis:
static-analysis: external-libs
@cd $(ROOT_DIR) && cppcheck -I $(INCLUDE_DIR) --enable=all $(SRC_DIR)

format:
Expand Down
60 changes: 32 additions & 28 deletions src/infoPanel/imagesCache.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include <SDL/SDL.h>
#include <SDL/SDL_image.h>
#include <SDL/SDL_rotozoom.h>

static SDL_Surface *g_image_cache_prev = NULL;
static SDL_Surface *g_image_cache_current = NULL;
Expand All @@ -15,42 +16,45 @@ static SDL_Surface *g_image_cache_next = NULL;
} while (0)
#endif

static void drawImage(SDL_Surface *image_to_draw, SDL_Surface *screen,
const SDL_Rect *frame)
#define MIN(a, b) (a < b) ? (a) : (b)

void drawImage(SDL_Surface *image, SDL_Surface *screen,
const SDL_Rect *frame)
{
if (!image_to_draw)
if (!image)
return;

if (image_to_draw->w != 640 || image_to_draw->h != 480) {
// scale image to 640x480 if needed
SDL_Rect dest_rect = {0, 0, 640, 480};
SDL_SoftStretch(image_to_draw, NULL, image_to_draw, &dest_rect);
image_to_draw->w = 640;
image_to_draw->h = 480;
}
SDL_Surface *scaled_image = NULL;
SDL_Rect target = {0, 0, screen->w, screen->h};

DEBUG_PRINT(("frame %p\n", frame));
int border_left = 0;
SDL_Rect new_frame = {0, 0};
if (frame != NULL) {
DEBUG_PRINT(("x-%d y-%d\n", frame->x, frame->y));
new_frame = *frame;
border_left = new_frame.x;
target = *frame;
}
DEBUG_PRINT(("border_left %d\n", border_left));
int16_t image_x = 320 - (int16_t)(image_to_draw->w / 2);
if (image_x < border_left) {
image_x = border_left;

// scale image to 640x480 only if bigger (v4 752x560)
if (image->w > target.w || image->h > target.h) {
double ratio_x = (double)target.w / image->w;
double ratio_y = (double)target.h / image->h;
double scale = MIN(ratio_x, ratio_y);
scaled_image = rotozoomSurface(image, 0.0, scale, true);

if (scaled_image == NULL) {
printf("rotozoomSurface failed: %s\n", SDL_GetError());
}
else {
image = scaled_image;
}
}
else {
new_frame.x -= border_left;

SDL_Rect image_pos = {
target.x + (target.w - image->w) / 2,
target.y + (target.h - image->h) / 2};

SDL_BlitSurface(image, NULL, screen, &image_pos);

if (scaled_image != NULL) {
SDL_FreeSurface(scaled_image);
}
SDL_Rect image_rect = {image_x, (int16_t)(240 - image_to_draw->h / 2)};
DEBUG_PRINT(("image_rect x-%d y-%d\n", image_rect.x, image_rect.y));
if (frame != NULL)
SDL_BlitSurface(image_to_draw, &new_frame, screen, &image_rect);
else
SDL_BlitSurface(image_to_draw, NULL, screen, &image_rect);
}

char *drawImageByIndex(const int new_image_index, const int image_index,
Expand Down
1 change: 1 addition & 0 deletions src/infoPanel/imagesCache.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ extern "C" {

#include <SDL/SDL.h>

void drawImage(SDL_Surface *image_to_draw, SDL_Surface *screen, const SDL_Rect *frame);
char *drawImageByIndex(const int index, const int image_index,
char **images_paths, const int images_paths_count,
SDL_Surface *screen, const SDL_Rect *frame,
Expand Down
49 changes: 30 additions & 19 deletions src/infoPanel/infoPanel.c
Original file line number Diff line number Diff line change
Expand Up @@ -102,16 +102,6 @@ static void drawInfoPanel(SDL_Surface *screen, SDL_Surface *video,
}
}

static void drawImage(const char *image_path, SDL_Surface *screen)
{
SDL_Surface *image = IMG_Load(image_path);
if (image) {
SDL_Rect image_rect = {320 - image->w / 2, 240 - image->h / 2};
SDL_BlitSurface(image, NULL, screen, &image_rect);
SDL_FreeSurface(image);
}
}

static void sdlQuit(SDL_Surface *screen, SDL_Surface *video)
{
SDL_FreeSurface(screen);
Expand All @@ -127,6 +117,16 @@ const SDL_Rect *getControlsAwareFrame(const SDL_Rect *frame)
return NULL;
}

void drawBackground(void)
{
if (g_show_theme_controls) {
SDL_BlitSurface(theme_background(), NULL, screen, NULL);
}
else {
SDL_FillRect(screen, NULL, 0);
}
}

int main(int argc, char *argv[])
{
char title_str[STR_MAX] = "";
Expand Down Expand Up @@ -180,20 +180,27 @@ int main(int argc, char *argv[])

bool cache_used = false;

const SDL_Rect themedFrame = {theme()->frame.border_left, 0, 640 - theme()->frame.border_right, 480};
const SDL_Rect themedFrame = {
theme()->frame.border_left, 60,
640 - theme()->frame.border_left - theme()->frame.border_right, 360};

SDL_Surface *static_image = NULL;

if (exists(image_path)) {
g_images_paths_count = 1;
g_image_index = 0;
SDL_BlitSurface(theme_background(), NULL, screen, NULL);
drawImage(image_path, screen);

drawBackground();

static_image = IMG_Load(image_path);
drawImage(static_image, screen, NULL);
}
else if (exists(images_json_path)) {
if (loadImagesPathsFromJson(images_json_path, &g_images_paths,
&g_images_paths_count, &g_images_titles) &&
g_images_paths_count > 0) {
g_image_index = 0;
SDL_BlitSurface(theme_background(), NULL, screen, NULL);
drawBackground();
drawImageByIndex(0, g_image_index, g_images_paths,
g_images_paths_count, screen,
getControlsAwareFrame(&themedFrame), &cache_used);
Expand All @@ -208,7 +215,7 @@ int main(int argc, char *argv[])
&g_images_paths_count) &&
g_images_paths_count > 0) {
g_image_index = 0;
SDL_BlitSurface(theme_background(), NULL, screen, NULL);
drawBackground();
drawImageByIndex(0, g_image_index, g_images_paths,
g_images_paths_count, screen,
getControlsAwareFrame(&themedFrame), &cache_used);
Expand Down Expand Up @@ -267,16 +274,16 @@ int main(int argc, char *argv[])
break;
case SW_BTN_Y:
g_show_theme_controls = !g_show_theme_controls;
SDL_BlitSurface(theme_background(), NULL, screen, NULL);
drawBackground();

if (g_images_paths) {
drawImageByIndex(
g_image_index, g_image_index, g_images_paths,
g_images_paths_count, screen,
getControlsAwareFrame(&themedFrame), &cache_used);
}
else if (exists(image_path)) {
drawImage(image_path, screen);
else if (static_image != NULL) {
drawImage(static_image, screen, NULL);
}
else {
g_show_theme_controls = true;
Expand Down Expand Up @@ -312,7 +319,7 @@ int main(int argc, char *argv[])

const int current_index = g_image_index;
navigating_forward ? g_image_index++ : g_image_index--;
SDL_BlitSurface(theme_background(), NULL, screen, NULL);
drawBackground();
drawImageByIndex(g_image_index, current_index, g_images_paths,
g_images_paths_count, screen,
getControlsAwareFrame(&themedFrame),
Expand Down Expand Up @@ -436,6 +443,10 @@ int main(int argc, char *argv[])

cleanImagesCache();

if (static_image != NULL) {
SDL_FreeSurface(static_image);
}

lang_free();
resources_free();
SDL_FreeSurface(screen);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
#!/bin/sh
echo ":: Gallery - Screenshots Viewer ::"

/mnt/SDCARD/.tmp_update/bin/infoPanel -d /mnt/SDCARD/Screenshots --show-theme-controls
infoPanel -d /mnt/SDCARD/Screenshots --show-theme-controls
ec=$?

# cancel or success from infoPanel
if [ $ec -eq 255 ] || [ $ec -eq 0 ] ; then
if [ $ec -eq 255 ] || [ $ec -eq 0 ]; then
exit 0
elif [ $ec -eq 1 ]; then
infoPanel -t Gallery -m "No screenshots found"
else
# something went wrong
infoPanel -t Gallery -m "An error occurred - code: $ec"
fi

/mnt/SDCARD/.tmp_update/bin/infoPanel -t Gallery -m "No screenshots found"
2 changes: 1 addition & 1 deletion static/packages/App/Quick Guide/App/Onion_Manual/launch.sh
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
#!/bin/sh
infoPanel --images-json /mnt/SDCARD/App/Onion_Manual/images.json --show-theme-controls
infoPanel --images-json /mnt/SDCARD/App/Onion_Manual/images.json
2 changes: 1 addition & 1 deletion test/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ CFILES := ../src/infoPanel/imagesCache.c
include ../src/common/config.mk

TARGET = test
LDFLAGS := $(LDFLAGS) -L../lib -s -lSDL_image -lSDL -lgtest -lgtest_main -lpthread
LDFLAGS := $(LDFLAGS) -L../lib -s -lSDL_image -lSDL -lSDL_rotozoom -lgtest -lgtest_main -lpthread

include ../src/common/commands.mk
include ../src/common/recipes.mk

0 comments on commit 575a70d

Please sign in to comment.