Skip to content
This repository has been archived by the owner on Apr 18, 2022. It is now read-only.

There is no way to display an image without a sprite sheet. #1086

Closed
CBenoit opened this issue Oct 29, 2018 · 21 comments
Closed

There is no way to display an image without a sprite sheet. #1086

CBenoit opened this issue Oct 29, 2018 · 21 comments
Labels
diff: normal Achievable by an reasonable experienced developer. If new to Amethyst, may need some guidance. pri: important Something other teams are relying on, or a low-level, critical piece of functionality. team: rendering

Comments

@CBenoit
Copy link
Member

CBenoit commented Oct 29, 2018

I investigated to build a game featuring a destructible world on a per pixel basis.
This requires to somehow get an ImageBuffer on the world texture. Using the current way of loading and accessing assets I could not find a way to get that.
Also, when the user only want to draw a texture there is no need for a full sprite sheet. A simple component holding the handle to the texture would suffice as the full texture should be rendered.

@Moxinilian
Copy link
Member

Moxinilian commented Oct 29, 2018

You can create a quad Mesh and add a Material to it with an associated Texture. It was the old way of displaying sprites.

@DoumanAsh
Copy link
Contributor

Also turns out there is dedicated UiImage

You can use it with transform to render some image (https://github.com/DoumanAsh/pong/blob/master/src/game/components/utils.rs#L17) for images, though it is a bit limit

@AnneKitsune
Copy link
Contributor

Mesh + Material is the way to go usually. I'm currently trying to find solutions so everything is using Material instead of having 3 different ways to render images

@Moxinilian
Copy link
Member

Moxinilian commented Oct 31, 2018

Material will not be suited for all sprite rendering FYI.

@AnneKitsune
Copy link
Contributor

How so?

@Moxinilian
Copy link
Member

Moxinilian commented Oct 31, 2018

Because you don't want a 2D workflow to reason in materials.

@eira-fransham
Copy link

Surely that's also an argument to have a simple way to deal with non-spritesheet images too, though? That you don't want a 2D workflow to have to deal with materials?

@zakarumych
Copy link
Member

Mesh is unnecessary overhead for sprite rendering.

@zakarumych
Copy link
Member

zakarumych commented Nov 2, 2018

You do want 42 ways to render images in 42 completely different situations if this means more performance.

@Moxinilian
Copy link
Member

@Vurich Ah yes totally. I thought I mentioned it here, but this should be improved in the new sprite pass with the new renderer.

@torkleyy
Copy link
Member

torkleyy commented Nov 4, 2018

@omni-viral While I think the possibility to do that is great, there should always be a way that is simple and just works.

@torkleyy torkleyy added diff: normal Achievable by an reasonable experienced developer. If new to Amethyst, may need some guidance. pri: important Something other teams are relying on, or a low-level, critical piece of functionality. team: rendering status: ready labels Nov 11, 2018
@happenslol
Copy link
Member

Hi! I'm super interested in working on this. Before I start and go off in the wrong direction, here's how I would go about it:

  • Make a component similar to SpriteRender, that works on an entire Texture instead of a SpriteSheet called TextureRender?)
  • Provide methods to easily build a TextureRender in the same way that a SpriteRender is built. I'd use chapters 7.2 to 7.4 of the book as a reference here and try to make the process as similar as possible.

Any thoughts on this? Would I be required to write tests/add to the documentation for this? I'd document the code itself of course.

@AnneKitsune
Copy link
Contributor

@happenslol I made a design for this. I'll be opening an rfc in a few hours. I will list the tasks inside of it, so you know what goes where if you want to work on it ;)

@Moxinilian
Copy link
Member

Moxinilian commented Nov 13, 2018

I don't think this requires an RFC. @happenslol's approach is what I would have advised to do, so I'd like to know what is your idea first.

@happenslol if you are going to do it, I'd recommend extending the current DrawSprite pass. I think you should be able to use the current shader as is on top of the spritesheet stuff.

@happenslol
Copy link
Member

I'm fine with either way, I just wanna make sure we don't do unnecessary work/anything twice. I'll have a look at @Jojolepro's RFC, no matter how I do it it will probably give me some insight since I haven't worked on this project yet. If I do start something, I'll do it tomorrow and check back here before.

@AnneKitsune
Copy link
Contributor

I didn't get to finish it yesterday. I'll try to publish it today.

@happenslol
Copy link
Member

I'm working on it now. So far I've only familiarized myself with the relevant parts of the engine and set up a simple example for testing. I'm calling the new type for a full texture Image for now, if you guys have any better ideas let me know. I see 2 ways of going about this:

  1. Implement a custom render pass for simple images. This would be a strongly simplified version of the DrawSprite pass, not drawing in batches and instead just rendering the texture out with the same shaders DrawSprite uses.
  2. Implement ImageRender on top of the DrawSprite pass, always just passing in one sprite without offsets that corresponds to the whole texture. This would probably result in a lot less code, but I don't know how much overhead it has and it doesn't feel intuitive to add a DrawSprite pass to render an image.

I'll probably try 2. and see how it feels. Any thoughts?

@AnneKitsune
Copy link
Contributor

I published the rfc here amethyst/rfcs#5

@happenslol
Copy link
Member

happenslol commented Nov 15, 2018

Ah, I see what you're going for. Makes sense to do that, but I feel like this is a little out of scope for this issue, since providing an easier way to render images is definitely a step in that direction, but more of a quick fix than a rework of the 2D passes in general.
I guess that means I'll just extend the DrawSprite pass (maybe rename it to something like DrawTexture pass to reflect that it does both now?) and then the rework can happen in a separate issue.

I also realized that extending the DrawSprite pass means that as long as we're drawing unordered, we're already getting batching for drawing multiple instances of the same full texture for free, since the sprites are sorting by their texture anyways. So that's a plus.

Is that alright with you guys?

@Moxinilian
Copy link
Member

Sounds good to me. Thanks!

@happenslol
Copy link
Member

I'm about done with the code itself, so I'm opening a PR for this. Not quite happy with the naming and I'm unsure how you guys handle "breaking" API changes (e.g. changing the naming for DrawSprite), but I think we can discuss that in the PR.

bors bot added a commit that referenced this issue Nov 17, 2018
1153: Implement a way of rendering images without spritesheets (Fixes #1086) r=jojolepro a=happenslol

A few things are still TODO here, I just wanted to open this up before I document all my changes.

As discussed in #1086, I've extended the `DrawSprite` pass to also take care of `ImageRender` structs, which allows the user to render full images without having to use configuration files or set up spritesheets. The same visibility sorting and filtering is used as for `Sprite`s. This also means that we get sorting and batching for free, so multiple images rendered from the same texture will not cause a state switch as long as they are drawn right after each other.

To reflect that the `DrawSprite` pass now renders both sprites and full images, I've renamed it to `DrawQuad`, and changed all mentions of it. I'm not quite happy with the name and I'm not sure we even want to change it, so I'm open to suggestions here. I'm also not sure if `SpriteVisibility` should be renamed too.

As for tests, I've looked at the tests for sprite and they only make sure that sprite coordinates are parsed correctly. For the pass there were no tests, and I'm only passing in new data structures into old code anyways, so I wasn't really sure how to add new tests for my changes specifically.

Things that are still left to do after the code has been reviewed and revised:

* [x] Update the relevant sections in the book
* [x] Make sure all examples are up to date with the new naming
* [x] Update the changelog

Co-authored-by: Hilmar Wiegand <me@hwgnd.de>
bors bot added a commit that referenced this issue Nov 17, 2018
1153: Implement a way of rendering images without spritesheets (Fixes #1086) r=jojolepro a=happenslol

A few things are still TODO here, I just wanted to open this up before I document all my changes.

As discussed in #1086, I've extended the `DrawSprite` pass to also take care of `ImageRender` structs, which allows the user to render full images without having to use configuration files or set up spritesheets. The same visibility sorting and filtering is used as for `Sprite`s. This also means that we get sorting and batching for free, so multiple images rendered from the same texture will not cause a state switch as long as they are drawn right after each other.

To reflect that the `DrawSprite` pass now renders both sprites and full images, I've renamed it to `DrawQuad`, and changed all mentions of it. I'm not quite happy with the name and I'm not sure we even want to change it, so I'm open to suggestions here. I'm also not sure if `SpriteVisibility` should be renamed too.

As for tests, I've looked at the tests for sprite and they only make sure that sprite coordinates are parsed correctly. For the pass there were no tests, and I'm only passing in new data structures into old code anyways, so I wasn't really sure how to add new tests for my changes specifically.

Things that are still left to do after the code has been reviewed and revised:

* [x] Update the relevant sections in the book
* [x] Make sure all examples are up to date with the new naming
* [x] Update the changelog

Fixes #1086 

Co-authored-by: Hilmar Wiegand <me@hwgnd.de>
@bors bors bot closed this as completed in #1153 Nov 17, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
diff: normal Achievable by an reasonable experienced developer. If new to Amethyst, may need some guidance. pri: important Something other teams are relying on, or a low-level, critical piece of functionality. team: rendering
Projects
None yet
Development

No branches or pull requests

8 participants