This change allows playlist icons to be replaced with Logos. #16758

jbreitweiser · 2024-07-05T14:56:26Z

Guidelines

Rebase before opening a pull request
If you are sending several unrelated fixes or features, use a branch and a separate pull request for each
If possible try squashing everything in a single commit. This is particularly beneficial in the case of feature merges since it allows easy bisecting when a problem arises

Description

This change allows playlist icons to be replaced with Logos. the art is loaded to the Thumbnail Named_Logos folder. It is treated the same as other thumbnails for naming and for automatic download. There is a settings menu option to turn it on and off. It is off by default. This only applies to the xmb menu driver.

Related Issues

[Any issues this pull request may be addressing]

Related Pull Requests

[Any other PRs from related repositories that might be needed for this pull request to work]

Reviewers

[If possible @mention all the people that should review your pull request]

…is loaded to the Thumbnail Named_Logos folder. It is treated the same as other thumbnails for naming and for automatic download. There is a settings menu option to turn it on and off. It is off by default. This only applies to the xmb menu driver.

gfx/gfx_thumbnail_path.c

hizzlekizzle · 2024-07-05T16:58:49Z

I think most of these CI build failures are unrelated. GLIBC stuff.

Do we want/need to supply these logos through the thumbnail server? is there a permissively-licensed source for them? Or do we assume people will have their own source?

LibretroAdmin · 2024-07-05T17:00:16Z

I think most of these CI build failures are unrelated. GLIBC stuff.

Do we want/need to supply these logos through the thumbnail server? is there a permissively-licensed source for them? Or do we assume people will have their own source?

I still can't get it to build though even re-running the tasks.

jbreitweiser · 2024-07-05T18:35:57Z

All of the errors reference GLIBC_2.28. Is there a problem with the build pipeline?

jbreitweiser · 2024-07-05T19:03:54Z

I think most of these CI build failures are unrelated. GLIBC stuff.
Do we want/need to supply these logos through the thumbnail server? is there a permissively-licensed source for them? Or do we assume people will have their own source?

I still can't get it to build though even re-running the tasks.

I actually tried to merge this yesterday and hit the GLIBC_2.28 error. So I deleted the branch, closed the pull request. Re forked Retroarch. Pulled down a new copy. Copied over the changes and then re committed and opened a pull request with the same results. I was thinking this might be a build pipeline issue.

jbreitweiser · 2024-07-05T19:10:36Z

I think most of these CI build failures are unrelated. GLIBC stuff.

Do we want/need to supply these logos through the thumbnail server? is there a permissively-licensed source for them? Or do we assume people will have their own source?

I was thinking this would be art added to the thumbnails download. I used the functions to pull down thumbnail art when it is not present. A user can add it themselves, that is how I tested, but i think in the long run it could be populated through the thumbnail server.

sonninnos · 2024-07-05T20:05:08Z

Nothing odd about these trivial errors.

menu/drivers/xmb.c: In function ‘xmb_unload_icon_thumbnail_textures’:
menu/drivers/xmb.c:1397:4: error: ‘for’ loop initial declarations are only allowed in C99 or C11 mode
 1397 |    for(size_t i =0; i < list_size ; i++)
      |    ^~~
menu/drivers/xmb.c:1397:4: note: use option ‘-std=c99’, ‘-std=gnu99’, ‘-std=c11’ or ‘-std=gnu11’ to compile your code
menu/drivers/xmb.c: In function ‘xmb_populate_dynamic_icons’:
menu/drivers/xmb.c:2274:7: error: C++ style comments are not allowed in ISO C90
 2274 |       //  Clear current textures if they are there
      |       ^
menu/drivers/xmb.c:2274:7: note: (this will be reported only once per input file)
menu/drivers/xmb.c:2293:13: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
 2293 |             xmb_icons_t *thumbnail_icon = &node->thumbnail_icon;
      |             ^~~~~~~~~~~
menu/drivers/xmb.c: In function ‘xmb_draw_item’:
menu/drivers/xmb.c:4372:7: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
 4372 |       bool show_icon_thumbnail          =
      |       ^~~~
cc1: some warnings being treated as errors
make: *** [Makefile:209: obj-unix/release/menu/drivers/xmb.o] Error 1

jbreitweiser · 2024-07-05T23:50:38Z

Nothing odd about these trivial errors.

menu/drivers/xmb.c: In function ‘xmb_unload_icon_thumbnail_textures’:
menu/drivers/xmb.c:1397:4: error: ‘for’ loop initial declarations are only allowed in C99 or C11 mode
 1397 |    for(size_t i =0; i < list_size ; i++)
      |    ^~~
menu/drivers/xmb.c:1397:4: note: use option ‘-std=c99’, ‘-std=gnu99’, ‘-std=c11’ or ‘-std=gnu11’ to compile your code
menu/drivers/xmb.c: In function ‘xmb_populate_dynamic_icons’:
menu/drivers/xmb.c:2274:7: error: C++ style comments are not allowed in ISO C90
 2274 |       //  Clear current textures if they are there
      |       ^
menu/drivers/xmb.c:2274:7: note: (this will be reported only once per input file)
menu/drivers/xmb.c:2293:13: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
 2293 |             xmb_icons_t *thumbnail_icon = &node->thumbnail_icon;
      |             ^~~~~~~~~~~
menu/drivers/xmb.c: In function ‘xmb_draw_item’:
menu/drivers/xmb.c:4372:7: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
 4372 |       bool show_icon_thumbnail          =
      |       ^~~~
cc1: some warnings being treated as errors
make: *** [Makefile:209: obj-unix/release/menu/drivers/xmb.o] Error 1

Cleaned up and committed

sonninnos · 2024-07-06T13:29:44Z

Could you show a screenshot of how this looks in practice?

And you still missed one: https://github.com/libretro/RetroArch/actions/runs/9819465190/job/27113026888?pr=16758

menu/drivers/xmb.c: In function ‘xmb_draw_item’:
menu/drivers/xmb.c:4390:10: error: C++ style comments are not allowed in ISO C90
 4390 |          //  Adjust icon location by recentering with half width and height
      |          ^
menu/drivers/xmb.c:4390:10: note: (this will be reported only once per input file)
make: *** [Makefile:209: obj-unix/release/menu/drivers/xmb.o] Error 1

jbreitweiser · 2024-07-06T13:37:25Z

Could you show a screenshot of how this looks in practice?

And you still missed one: https://github.com/libretro/RetroArch/actions/runs/9819465190/job/27113026888?pr=16758
menu/drivers/xmb.c: In function ‘xmb_draw_item’:
menu/drivers/xmb.c:4390:10: error: C++ style comments are not allowed in ISO C90
 4390 |          //  Adjust icon location by recentering with half width and height
      |          ^
menu/drivers/xmb.c:4390:10: note: (this will be reported only once per input file)
make: *** [Makefile:209: obj-unix/release/menu/drivers/xmb.o] Error 1

Comment removed

zoltanvb · 2024-07-07T16:19:40Z

Tested it a bit, works nicely, but I do not see any download attempts when using the on-demand thumbnail downloader or the playlist thumbnail downloader. (That can be added later, anyways.)

What is the optimal size for the logos? I threw in some portrait images for testing, and ofc they look off, but if this gets merged and some enthusiastic people start to fill the thumbnail repos, it would be nice to have a sort of standard.

jbreitweiser · 2024-07-07T17:52:00Z

Tested it a bit, works nicely, but I do not see any download attempts when using the on-demand thumbnail downloader or the playlist thumbnail downloader. (That can be added later, anyways.)

What is the optimal size for the logos? I threw in some portrait images for testing, and ofc they look off, but if this gets merged and some enthusiastic people start to fill the thumbnail repos, it would be nice to have a sort of standard.

So it should be resizing all images to 256 x 256 for the selected ROM. The non selected entries are 234 x 234. All the images I used were wider than taller. The resize function uses the width in proportion to the height. So if the height is greater than the width it might be causing the overlap.

jbreitweiser · 2024-07-10T11:53:20Z

Is there a way to fix the 3DS build? The glibc seems to be fixed for the other builds but that one continues to fail.

zoltanvb · 2024-07-17T14:21:37Z

Try a rebase, 3DS CI works since d05b319 .

hizzlekizzle · 2024-07-18T22:35:12Z

eyy, nice. all checks have passed.

@LibretroAdmin did you have any further requests?

gfx/gfx_display.c

LibretroAdmin · 2024-07-25T01:28:07Z

gfx/gfx_thumbnail_path.c

+   settings_t *settings       = config_get_ptr();
+
+   if (!path_data)
+   {


For single lines, the brackets are not necessary

code reformatted

LibretroAdmin · 2024-07-25T01:28:18Z

gfx/gfx_thumbnail_path.c

+   path_data->playlist_index          = 0;
+
+   if (!playlist)
+   {


For single lines, the brackets are not necessary

code reformatted

LibretroAdmin · 2024-07-25T01:28:22Z

gfx/gfx_thumbnail_path.c

+   }
+
+   if (idx >= playlist_get_size(playlist))
+   {


For single lines, the brackets are not necessary

code reformatted

LibretroAdmin · 2024-07-25T01:28:40Z

gfx/gfx_thumbnail_path.c

+         char tmp_buf[PATH_MAX_LENGTH];
+         const char* pos = strchr(db_name, '|');
+
+         if (pos && (size_t) (pos - db_name)+1 < sizeof(tmp_buf)) {


Opening bracket on a newline

code reformatted

LibretroAdmin · 2024-07-25T01:28:48Z

gfx/gfx_thumbnail_path.c

+             * databases mentioned separated by |, use only first one */
+            strlcpy(tmp_buf, db_name, (size_t) (pos - db_name)+1);
+         }
+         else {


Opening bracket on a newline

LibretroAdmin · 2024-07-25T01:29:44Z

menu/drivers/xmb.c

+   {
+      xmb_node_t *node = (xmb_node_t*)selection_buf->list[i].userdata;
+      if(node)
+      {


Brackets not necessary for a single line

code reformatted

change strcpy to strlcpy

fix code formatting

code formatting changes

jbreitweiser · 2024-07-26T03:48:16Z

Al requested changes complete and repo rebased. All checks passed.

@LibretroAdmin did you have any further requests?

LibretroAdmin · 2024-08-07T15:39:44Z

Sorry for the long wait. Merged.

…o#16758) * This change allows playlist icons to be replaced with Logos. the art is loaded to the Thumbnail Named_Logos folder. It is treated the same as other thumbnails for naming and for automatic download. There is a settings menu option to turn it on and off. It is off by default. This only applies to the xmb menu driver. * Removed commented out code against the style guide. * Code cleanup for C89 compatibitity * Cleaned up errors from Automated CI. * Cleaned up comments. * Update gfx_display.c change strcpy to strlcpy * Update gfx_thumbnail_path.c fix code formatting * Update xmb.c code formatting changes

OctopusButtons · 2025-01-01T20:47:33Z

Nice work on this. I always imagined someday setting actual game disc picture (rather than logo per se), for select favorite PS1 playlist games instead of just the generic disc icon.

LibretroAdmin requested changes Jul 5, 2024

View reviewed changes

gfx/gfx_thumbnail_path.c Outdated Show resolved Hide resolved

Removed commented out code against the style guide.

ec5aaf8

Code cleanup for C89 compatibitity

f73194d

jbreitweiser and others added 2 commits July 5, 2024 20:16

Cleaned up errors from Automated CI.

e0201c7

Merge branch 'libretro:master' into master

fe503a2

Cleaned up comments.

0e8d75d

Merge branch 'libretro:master' into master

dd4764c

LibretroAdmin requested changes Jul 25, 2024

View reviewed changes

jbreitweiser added 4 commits July 25, 2024 22:21

Update gfx_display.c

5e612eb

change strcpy to strlcpy

Update gfx_thumbnail_path.c

7bffa5f

fix code formatting

Update xmb.c

7a6406b

code formatting changes

Merge branch 'libretro:master' into master

029c681

LibretroAdmin merged commit 11d9a84 into libretro:master Aug 7, 2024
27 checks passed

zoltanvb mentioned this pull request Sep 6, 2024

Invoke menu icon thumbnail loading only if needed. #16966

Merged

zoltanvb mentioned this pull request Sep 13, 2024

Add mention of Named_Logos and non-PNG thumbnails. libretro/docs#990

Merged

zach-morris mentioned this pull request Nov 27, 2024

Add Named_Logos #17216

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

This change allows playlist icons to be replaced with Logos. #16758

This change allows playlist icons to be replaced with Logos. #16758

jbreitweiser commented Jul 5, 2024

hizzlekizzle commented Jul 5, 2024

LibretroAdmin commented Jul 5, 2024

jbreitweiser commented Jul 5, 2024

jbreitweiser commented Jul 5, 2024

jbreitweiser commented Jul 5, 2024

sonninnos commented Jul 5, 2024

jbreitweiser commented Jul 5, 2024

sonninnos commented Jul 6, 2024 •

edited

Loading

jbreitweiser commented Jul 6, 2024

zoltanvb commented Jul 7, 2024

jbreitweiser commented Jul 7, 2024 •

edited

Loading

jbreitweiser commented Jul 10, 2024

zoltanvb commented Jul 17, 2024

hizzlekizzle commented Jul 18, 2024

LibretroAdmin Jul 25, 2024

jbreitweiser Jul 26, 2024

LibretroAdmin Jul 25, 2024

jbreitweiser Jul 26, 2024

LibretroAdmin Jul 25, 2024

jbreitweiser Jul 26, 2024

LibretroAdmin Jul 25, 2024

jbreitweiser Jul 26, 2024

LibretroAdmin Jul 25, 2024

LibretroAdmin Jul 25, 2024

jbreitweiser Jul 26, 2024

jbreitweiser commented Jul 26, 2024

LibretroAdmin commented Aug 7, 2024

OctopusButtons commented Jan 1, 2025

This change allows playlist icons to be replaced with Logos. #16758

This change allows playlist icons to be replaced with Logos. #16758

Conversation

jbreitweiser commented Jul 5, 2024

Guidelines

Description

Related Issues

Related Pull Requests

Reviewers

hizzlekizzle commented Jul 5, 2024

LibretroAdmin commented Jul 5, 2024

jbreitweiser commented Jul 5, 2024

jbreitweiser commented Jul 5, 2024

jbreitweiser commented Jul 5, 2024

sonninnos commented Jul 5, 2024

jbreitweiser commented Jul 5, 2024

sonninnos commented Jul 6, 2024 • edited Loading

jbreitweiser commented Jul 6, 2024

zoltanvb commented Jul 7, 2024

jbreitweiser commented Jul 7, 2024 • edited Loading

jbreitweiser commented Jul 10, 2024

zoltanvb commented Jul 17, 2024

hizzlekizzle commented Jul 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbreitweiser commented Jul 26, 2024

LibretroAdmin commented Aug 7, 2024

OctopusButtons commented Jan 1, 2025

sonninnos commented Jul 6, 2024 •

edited

Loading

jbreitweiser commented Jul 7, 2024 •

edited

Loading