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

Bugfix: Dynamically generated texture buffers have the wrong size #1144

Merged
merged 1 commit into from Nov 22, 2018

Conversation

Projects
None yet
4 participants
@Stoeoef
Copy link
Contributor

Stoeoef commented Nov 14, 2018

Hi there!
I've stumbled upon an issue when trying to create a dynamic texture (that is, not loaded from a file). The size of the opengl texture buffer for non-image textures is always 1x1 (that's why defining a texture with only one color works), but custom sizes defined via TextureMetadata are 'forgotten'.

I've never done any serious error handling in my other projects, so please have a look at the error that I use to indicate that a texture of invalidate size is created. Also, the code does not Err if "too many" pixels are defined... that probably also should result in an error.

@Stoeoef Stoeoef force-pushed the Stoeoef:master branch 2 times, most recently from 6f719c2 to 9b92845 Nov 14, 2018

@torkleyy
Copy link
Member

torkleyy left a comment

Thanks for the fix!

Show resolved Hide resolved amethyst_renderer/src/tex.rs Outdated
Show resolved Hide resolved amethyst_renderer/src/tex.rs Outdated
@torkleyy
Copy link
Member

torkleyy left a comment

Ah sorry, you addressed just that in your PR comment! So for the error I think there should be something like PixelDataMismatch that contains both the expected and the actual size.

@Stoeoef Stoeoef force-pushed the Stoeoef:master branch from 9b92845 to 0d15f57 Nov 15, 2018

@Stoeoef

This comment has been minimized.

Copy link
Contributor

Stoeoef commented Nov 15, 2018

Thanks for the feedback!
I've updated the PR accordingly.

@Rhuagh

Rhuagh approved these changes Nov 16, 2018

Copy link
Member

Rhuagh left a comment

Just make sure to run through all the examples that load textures so we know atleast all those still work.

@jojolepro

This comment has been minimized.

Copy link
Member

jojolepro commented Nov 16, 2018

Name suggestion: PixelSizeMismatch

@torkleyy
Copy link
Member

torkleyy left a comment

Thank you!

@torkleyy

This comment has been minimized.

Copy link
Member

torkleyy commented Nov 17, 2018

Can somebody confirm the examples all work?

@Stoeoef

This comment has been minimized.

Copy link
Contributor

Stoeoef commented Nov 18, 2018

I've tested most examples, all look fine. The gltf example did not work properly, however, a rebase onto the current master solved this - so I guess that is related to other changes.
All examples seem to either load an image (which is unaffected by this change) or used a single color as texture, which "accidentally" worked before, as they all had dimensions 1x1.

@torkleyy
Copy link
Member

torkleyy left a comment

Thank you for the fix, merging!

bors r=torkleyy,Rhuagh

bors bot added a commit that referenced this pull request Nov 22, 2018

Merge #1144
1144: Bugfix: Dynamically generated texture buffers have the wrong size r=torkleyy,Rhuagh a=Stoeoef

Hi there!
I've stumbled upon an issue when trying to create a dynamic texture (that is, not loaded from a file). The size of the opengl texture buffer for non-image textures is always 1x1 (that's why defining a texture with only one color works), but custom sizes defined via `TextureMetadata` are 'forgotten'.

I've never done any serious error handling in my other projects, so please have a look at the error that I use to indicate that a texture of invalidate size is created. Also, the code does not `Err` if "too many" pixels are defined... that probably also should result in an error.

Co-authored-by: Stefan Altmayer <stoeoef@gmail.com>
@bors

This comment has been minimized.

Copy link
Contributor

bors bot commented Nov 22, 2018

@bors bors bot merged commit 0d15f57 into amethyst:master Nov 22, 2018

4 checks passed

bors Build succeeded
Details
concourse-ci/linux-book-tests Concourse CI build success
Details
concourse-ci/linux-tests Concourse CI build success
Details
concourse-ci/rustfmt Concourse CI build success
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment