fix logging related to shader loading

[sneakymizu]

Changelog

• Category: Bug fixes
• Description:
  • Fixed check for error when loading shaders via fg-window-rules (inverted condition)
  • I think the opened shader files are not properly closed when successfully read, so I added a fclose for the happy path

[Swivelgames]

load_shader_source uses inverted boolean logic, so it returns true in the case of errors.

This change would break the logging related to shader loading, unless load_shader_source was updated to always return true, and return false in the case of errors.

I'm not recommending that, of course, but I am curious: Is there are particular issue that you're running into that warranted this PR? Or was this found while perusing the codebase?

[sneakymizu]

load_shader_source uses inverted boolean logic, so it returns true in the case of errors.

So this would mean, that the logmessage should display if load_shader_source is true, which is the change here. In theory it would be nice to improve the logging here on what shader failed to load, but as I'm just dipping my toes I'm cautions in suggesting larger changes. (e.g. I found some missing nullpointer checks in the dbus api which causes picom to segfault. I've suggestions for that but waiting on feedback on this for now)

I was checking how the shaders are integrated in picom and how to configure them. As I understand it, it would only change the behaviour of logging that a shader could not be loaded due to an error, when loading multiple shaders instead of stating that there was an error with the shader even if there is none.

Putting it in other words: I was (initially) toying with the shaders and was irritated when I saw an error but the shader was properly displaying. Though before fixing my shader I was not seeing this error message. I was checking on the shader integration in general and found that the boolean check for loading multiple shaders is invers to the one when loading only the window_shader_fg shader. Thus I assumed this was an oversight. I might be wrong, but for me that seems plausible.

I don't need this change, I just thought it might be nice to fix something (even minor) when I see it. That is, if my understanding is correct.

[sneakymizu]

For comparison picom v12.5

With this change:

The shaders configured in the picom config are used both times - I've terminated picom for the screenshots though. For these functional shaders in the first screenshot picom v12.5 (upstream) states for each, that they are erroneous. In the updated version these messages are not printed - as they are technically false.

[Swivelgames]

Ah, no, you're very correct! My apologies, I tripped over myself. Carry on! :sweat_smile:

[yshui]

thanks for the fixes, can you rebase your branch on top of next, instead of merging it?

[sneakymizu]

sure. github provided a shiny button :D

[absolutelynothelix]

@l4desu-mizu, every shiny button github provides is a trap

[absolutelynothelix]

superseded by cb6993f5a0592af95007df3c458cfa6073ed5bde and a8ef63dbc7543d2951c1a204732f1fe9189aa93c.

thanks for bringing this to our attention!