Skip to content

vello_hybrid: Adjust atlas config to actual device capabilities#1568

Merged
LaurenzV merged 1 commit into
mainfrom
laurenz/atlas_config
Apr 14, 2026
Merged

vello_hybrid: Adjust atlas config to actual device capabilities#1568
LaurenzV merged 1 commit into
mainfrom
laurenz/atlas_config

Conversation

@LaurenzV
Copy link
Copy Markdown
Collaborator

@LaurenzV LaurenzV commented Apr 13, 2026

Right now, AtlasConfig implements Default with the following configuration:

impl Default for AtlasConfig {
    fn default() -> Self {
        Self {
            initial_atlas_count: 1,
            max_atlases: 8,
            atlas_size: (4096, 4096),
            auto_grow: true,
            allocation_strategy: AllocationStrategy::FirstFit,
        }
    }
}

Given that this is part of RenderSettings, users should be able to assume that they can do RenderSettings::default and it just works everywhere. However, this is currently not the case, as we will blindly try to allocate the resources without checking the actual device capabilties.

This PR proposes to change this by treating the config as a strong recommendation, but allowing backends to choose different values based on the capabilities of the device it runs on. I experimented with another approach where we instead introduce an Auto struct for some fields to allow the users to explicitly opt in for that, and instead let the backend error out in case the user-requested parameters are not compatible with the device. However, this resulted in a much larger code diff, hence why I think just mutating the settings is the easier approach for now.

@LaurenzV LaurenzV marked this pull request as draft April 13, 2026 08:36
@LaurenzV LaurenzV marked this pull request as ready for review April 13, 2026 08:44
@LaurenzV LaurenzV requested a review from taj-p April 13, 2026 08:44
@LaurenzV LaurenzV added this pull request to the merge queue Apr 14, 2026
Merged via the queue into main with commit f8303ee Apr 14, 2026
17 checks passed
@LaurenzV LaurenzV deleted the laurenz/atlas_config branch April 14, 2026 04:57
max_texture_dimension_2d,
get_max_texture_array_layers(&gl),
1,
);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The change in this PR is a bit controversial. Consider testing the renderer against edge cases to ensure it properly handles failure modes. Also, it now hides from the consumer that its configuration has been adjusted based on device capabilities.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Would you prefer a different implementation?

Copy link
Copy Markdown
Collaborator

@grebmeg grebmeg May 6, 2026

Choose a reason for hiding this comment

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

It’s fine to leave this as it is for now, but whenever we’re working near this functionality, I’d consider updating it to make the config changes more explicit (or rather consider it's as setup-config) instead of silently applying them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants