Skip to content

Added gradient example#631

Merged
dhardy merged 2 commits intokas-gui:masterfrom
Kwarrtz:push-qoouvwvtlzyr
Feb 11, 2026
Merged

Added gradient example#631
dhardy merged 2 commits intokas-gui:masterfrom
Kwarrtz:push-qoouvwvtlzyr

Conversation

@Kwarrtz
Copy link
Contributor

@Kwarrtz Kwarrtz commented Jan 6, 2026

This adds an example using Sprite to the existing PR #627

@Kwarrtz
Copy link
Contributor Author

Kwarrtz commented Jan 6, 2026

I'm also happy to add comments and a screenshot and so on, but I wanted to see if you had any thoughts first.

Copy link
Collaborator

@dhardy dhardy left a comment

Choose a reason for hiding this comment

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

Thanks for the example. I'll update my PR. (Looks like Sprite::allocate is unnecessary.)

@Kwarrtz
Copy link
Contributor Author

Kwarrtz commented Jan 7, 2026

Rendering here is unnecessary if the new size matches the existing size. Sprite already caches the existing size; I'll make it available.

Oops, yes, I meant to check that and just forgot.

Rendering shouldn't happen if already in progress (hence why I used a state enum). Kas often calls set_rect twice on start (it's to do with window sizing and maybe / maybe not fixable). Window resizes may happen faster than the re-render speed (can cause lag & high CPU usage).

Never triggering a rerender when one is already in progress could leave the image out of sync with the size of the Sprite. E.g., if we call set_rect(rect1) and then, before the render is complete, set_rect(rect2), the buffer will be stuck with size rect1 (until a new call to set_rect). I think my current implementation avoids both this and unnecessary rerenders, but are superfluous calls to set_rect really common enough to make this worth it?

Also, to fully optimize this, we would want some way to cancel in progress rendering when a resize makes it obsolete. Should send_spawn return some kind of task handle to allow that?

It doesn't verify the image size matches the allocation. I'll fix this.

Why does this require us to pass the size into image_upload? Shouldn't we just check data.len() against image_size(handle)?

Comment on lines 145 to 149
if let Some(NewBuffer { buffer, size }) = cx.try_pop() {
let draw = cx.draw_shared();
if let Some(handle) = self.sprite.handle()
&& draw.image_size(handle) == Some(size)
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

The size test is less important now (image_upload will fail if it doesn't match the allocation).

But what isn't handled here is the case rendering was already in progress when set_rect was last called, and thus size != self.sprite.rect().size, so that needs handling (optionally upload this result anyway).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We still need to re-allocate if the size of the image has changed. Removing that check won't cause a panic but it will cause the upload to repeatedly fail and so the image won't update.

Copy link
Collaborator

Choose a reason for hiding this comment

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

You're comparing the size used by the render buffer to the allocation. There's a third size: sprite.rect().size.

@dhardy
Copy link
Collaborator

dhardy commented Jan 7, 2026

Never triggering a rerender when one is already in progress could leave the image out of sync with the size of the Sprite.

Svg and Canvas handle this by checking size again when receiving a render result.

but are superfluous calls to set_rect really common enough to make this worth it?

At least on my system (Wayland/Linux) resize events can happen rather quickly. When it works right, resizing the window feels very responsive, but it can also cause excessive load.

Also, to fully optimize this, we would want some way to cancel in progress rendering when a resize makes it obsolete. Should send_spawn return some kind of task handle to allow that?

I don't think this matters. Even if the result is outdated, it's probably more accurate than the previous one, so e.g. Svg will upload anyway.

Hmm, futures can be "cancelled" by just dropping them (though "cancel safety" depends on the futures). I haven't provided any way of doing that; not sure if I should to be honest. It's still possible for long-running tasks to use shared state to check whether they should terminate themselves.

@dhardy
Copy link
Collaborator

dhardy commented Jan 7, 2026

Why does this require us to pass the size into image_upload? Shouldn't we just check data.len() against image_size(handle)?

Theoretically size.0 * size.1 could still match data.len() while being wrong, but this is besides the most important point: data.len() must match. This is currently only checked with an assert.

So go ahead and remove the size parameter if you like and replace the assert_eq on data.len with something like return Err(UploadError::DataLen).

@dhardy
Copy link
Collaborator

dhardy commented Jan 7, 2026

You'll also need to check --features soft since there are two rendering backends.

@Kwarrtz Kwarrtz marked this pull request as draft January 7, 2026 15:05
@Kwarrtz
Copy link
Contributor Author

Kwarrtz commented Jan 7, 2026

Converted to draft because I need to spend a bit more time looking at the soft backend to figure out how best to integrate with its existing error-handling scheme. I don't think the previous commit is the best choice.

Comment on lines 633 to 646
SpriteType::Bitmap => self.atlas_rgba.upload(atlas, origin, size, &data),
}
}?;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

These tex uploads (from text glyph rastering) shouldn't fail, but in case they do this shouldn't stop the remaining uploads. Possibly best to catch-and-ignore here with a note about the cause having been logged.

@dhardy
Copy link
Collaborator

dhardy commented Feb 9, 2026

@Kwarrtz would you like to rebase your example? The changes to Kas crates are no longer needed.

@Kwarrtz Kwarrtz changed the base branch from push-qoouvwvtlzyr to master February 11, 2026 12:18
@Kwarrtz Kwarrtz marked this pull request as ready for review February 11, 2026 12:50
@Kwarrtz
Copy link
Contributor Author

Kwarrtz commented Feb 11, 2026

Done. The example itself is pretty much unchanged from before, just had to move the image format argument from upload to allocate. Seems to work fine on both backends.

Copy link
Collaborator

@dhardy dhardy left a comment

Choose a reason for hiding this comment

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

LGTM thank you.

@dhardy dhardy merged commit e48639b into kas-gui:master Feb 11, 2026
5 checks passed
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.

2 participants

Comments