only add circuitpython_splash to display if it's not in another group #7829
Conversation
| @@ -136,7 +136,9 @@ void common_hal_displayio_display_construct(displayio_display_obj_t *self, | |||
|
|
|||
| // Set the group after initialization otherwise we may send pixels while we delay in | |||
| // initialization. | |||
| common_hal_displayio_display_set_root_group(self, &circuitpython_splash); | |||
| if (!circuitpython_splash.in_group) { | |||
There was a problem hiding this comment.
This code looks fine, but I noticed a few lines later it checks if root_group is NULL and if so assigns it to circuitpython_splash with no checks and not using the function. Not sure how that may affect anything. But your check on its own looks good.
There was a problem hiding this comment.
Are you talking about when calling display.show(None) ? It uses common_hal_displayio_display_show which does a few checks, including if the group is already in a group, and returns False when those checks fail. That return is checked and an exception is raised in the shared-bindings part:
circuitpython/shared-bindings/displayio/Display.c
Lines 261 to 264 in 4e7bbf7
Maybe it could use
common_hal_displayio_display_set_root_group instead. (show() is deprecated anyway).
|
The latest commit adds the same check as the original commit but to a different section of code inside reset_display() which had caused trouble raised by #7472 |
resolves #7824
Tested successfully on Monster M4sk
Not sure if we need to guard more cautiously against
circuitpython_splashbeing null or not havingin_group. This code assumes it will exist and have that property to check. It does seem safe after testing on Monster M4sk but I don't know if there are other scenarios that could be problematic.