Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fallback to background color when background image not found #46

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions src/renderer/AsyncResourceGatherer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <pango/pangocairo.h>
#include <algorithm>
#include "../core/hyprlock.hpp"
#include "../helpers/Log.hpp"

std::mutex cvmtx;

Expand Down Expand Up @@ -61,12 +62,18 @@ void CAsyncResourceGatherer::gather() {
if (path.empty())
continue;

std::string id = std::string{"background:"} + path;

// preload bg img
const auto CAIROISURFACE = cairo_image_surface_create_from_png(path.c_str());

const auto CAIRO = cairo_create(CAIROISURFACE);
// Make sure the img was actually loaded
if (cairo_surface_status(CAIROISURFACE) != CAIRO_STATUS_SUCCESS) {
Debug::log(WARN, "Background image could not be loaded: {}", path);
continue;
}

std::string id = std::string{"background:"} + path;

const auto CAIRO = cairo_create(CAIROISURFACE);
cairo_scale(CAIRO, 1, 1);

const auto TARGET = &preloadTargets.emplace_back(CAsyncResourceGatherer::SPreloadTarget{});
Expand Down
13 changes: 5 additions & 8 deletions src/renderer/widgets/Background.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,18 @@ CBackground::CBackground(const Vector2D& viewport_, const std::string& resourceI
}

bool CBackground::draw(const SRenderData& data) {
if (!asset)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually, I'd check for the resourceID being empty here too. getAssetByID does a lookup and if the resourceID is empty why would we lookup

Copy link
Author

@DerekSauer DerekSauer Feb 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The resourceID isn't empty.

In the resource gatherer if the image cannot be found on disk it skips assigning an ID and bails early just like if the background path in the config was empty since there isn't actually a resource to be found.

In CRenderer::getOrCreateWidgetsFor() it uses background: + PATH as the resource ID. PATH does contain a valid value (the path where we expected the resource to exist on disk) but no associate resource ever got loaded.

Checking for an empty ID will always return true since there is an ID, its just looking for a resource that was never there.

Copy link
Author

@DerekSauer DerekSauer Feb 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another approach would be to add an additional check in CRenderer::getOrCreateWidgetsFor() to see if the desired file exists and push an empty string ("") as the resource ID if it does not. That would allow checking for an empty resource ID in background render and skip the resource lookup, which is wasteful since the resource will never exist anyway.

I didn't do it that way since I wanted to error check the PNG load to guard not only against missing files but also corrupted files and OOM and such. Cairo has a bunch of error states it can return when loading an image.

Gonna try something.

asset = g_pRenderer->asyncResourceGatherer->getAssetByID(resourceID);

if (resourceID.empty()) {
// If background image does not exist, for whatever reason, fallback to color
if (!asset) {
CBox monbox = {0, 0, viewport.x, viewport.y};
CColor col = color;
col.a *= data.opacity;
g_pRenderer->renderRect(monbox, col, 0);
return data.opacity < 1.0;
}

if (!asset)
asset = g_pRenderer->asyncResourceGatherer->getAssetByID(resourceID);

if (!asset)
return false;

CBox texbox = {{}, asset->texture.m_vSize};

Vector2D size = asset->texture.m_vSize;
Expand All @@ -38,4 +35,4 @@ bool CBackground::draw(const SRenderData& data) {
g_pRenderer->renderTexture(texbox, asset->texture, data.opacity);

return data.opacity < 1.0;
}
}
Loading