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

Setting a Negative Scale for a Sprite Makes it Invisible #1385

Closed
zicklag opened this issue Feb 2, 2021 · 11 comments
Closed

Setting a Negative Scale for a Sprite Makes it Invisible #1385

zicklag opened this issue Feb 2, 2021 · 11 comments

Comments

@zicklag
Copy link
Member

zicklag commented Feb 2, 2021

Bevy version

4796ea8

Operating system & version

Pop!_OS ( Ubuntu ) 20.04

What you did

Spawn a sprite in a Bevy game with an X scale of -1.0:

use bevy::prelude::*;

fn main() {
    App::build()
        .add_plugins(DefaultPlugins)
        .add_startup_system(setup.system())
        .run();
}

fn setup(
    commands: &mut Commands,
    asset_server: Res<AssetServer>,
    mut materials: ResMut<Assets<ColorMaterial>>,
) {
    let texture_handle = asset_server.load("branding/icon.png");
    commands
        // .spawn(OrthographicCameraBundle::new_2d())
        .spawn(OrthographicCameraBundle::new_2d())
        .spawn(SpriteBundle {
            material: materials.add(texture_handle.into()),
            transform: Transform::from_scale(Vec3::new(-1.0, 1.0, 1.0)),
            ..Default::default()
        });
}

What you expected to happen

The sprite should be displayed flipped along the X axis. At least that is the behavior I observed recently. The most recent commit I know of exhibiting the flipped behavior is bff44f7. Maybe it is not intended behavior to flip sprites by setting a negative scale?

What actually happened

The sprite was completely invisible.

@bjorn3
Copy link
Contributor

bjorn3 commented Feb 2, 2021

I think backface culling is enabled.

@zicklag
Copy link
Member Author

zicklag commented Feb 2, 2021

Ah, that would make sense. It looks like PipelineDescriptor at least got some changes and such so it probably came with those. I think I could make a PR for adding a flip parameter to the Sprite component if that would be a more reasonable way to do it. I actually feel that that is more semantically correct that negative scale anyay. 🤔

@rparrett
Copy link
Contributor

rparrett commented Feb 5, 2021

I think that this may be a regression from 81809c7.

I am seeing the same behavior with what I believe was the "official way" to flip a sprite (rotating 180 deg over y) after that commit.

@rparrett
Copy link
Contributor

rparrett commented Feb 5, 2021

It looks like sprites lost their CullMode::None in that commit

@zicklag
Copy link
Member Author

zicklag commented Feb 5, 2021

I am just about to start tryout out a flip parameter to the Sprite component. I think that will make more sense than setting a - scale or a 180 rotation from a user experience perspective. I feel like that makes more sense as an "official way", but I'd like to hear other ideas if there are any.

@rparrett
Copy link
Contributor

rparrett commented Feb 5, 2021

I tested out giving CullMode::None back to sprites, and that seems to work okay. I'll reach out in discord for a second opinion. I suspect that we may want CullMode::None even if we have a really ergonomic way to actually flip the texture or whatever. (Which I personally think would be great)

@cart
Copy link
Member

cart commented Feb 5, 2021

I think under the hood our options are:

  1. Draw with a flipped transform and no culling
  2. "flip" by assigning a "flipped" quad mesh (and culling ceases to matter)
  3. Go "meshless" by just issuing commands to draw 4 vertices. Vertex positions would then be looked up from constants in the shader. We could "flip" by changing the order vertices are returned in. (I haven't tried the flipping part before, but I've done this for sprites in the past)
  4. Actually flip the texture (not viable due to double allocations and reduction in batch-ability)

I think (1) is the easiest fix and preferable in the short term. (2) reduces our ability to batch draws (as we need to rebind the mesh). (3) might be good, but would require more investigation. I'd rather tackle that question when we implement batching. (4) is a complete non-starter.

@cart
Copy link
Member

cart commented Feb 5, 2021

Just merged #1399. I'll close this for now, but feel free to keep discussing impls here if you're interested.

@cart cart closed this as completed Feb 5, 2021
@zicklag
Copy link
Member Author

zicklag commented Feb 5, 2021

I've flipped textures trivially in my tile map renderer by flipping the UV. That way it doesn't have to change the geometry at all, and all the logic goes in the fragment shader. I think that would be easily possible here, too. I think I'll get a chance to try that out tomorrow.

@cart
Copy link
Member

cart commented Feb 5, 2021

Ah yeah thats definitely an important one to call out. When drawing a single image, you can just negate the x or y value and (provided the sampler repeats), it will flip. However for sprite sheets things get a little more interesting. Wouldn't you'd need to pass in additional information to the shader (current sprite dimensions and start point) to do the flip? That would be more expensive and less batchable.

@zicklag
Copy link
Member Author

zicklag commented Feb 5, 2021

@cart Yeah, I'm not sure how it would effect performance, but I did manage to get an implementation that required adding only one uint of data to the shader resources: #1407. There's a slight render glitch that I'm not sure how to fix, but hopefully somebody can give me a hint on that.

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

No branches or pull requests

4 participants