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.

Sign up for GitHub

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

Jump to bottom

Image texture code needs some love #434

Closed
hollasch opened this issue Apr 2, 2020 · 1 comment
Closed

Image texture code needs some love #434

hollasch opened this issue Apr 2, 2020 · 1 comment
Assignees
@hollasch
Labels
area: book area: code
Milestone
v3.1.0

Comments

@hollasch
Copy link
Collaborator

hollasch commented Apr 2, 2020

u is mapped to [0, image_width] instead of [0,image_width-1]. Similarly for v, except that it then subtracts 0.001 for some reason.

The subsequent bounds clipping then corrects, but the last horizontal/vertical pixels are lost at this point. Better to clip the U,V coordinates and then apply a correct real→integer transform.

The pixel base address is recomputed three times — it should be calculated in the constructor instead of three times for every pixel access.
@hollasch hollasch added this to the v3.1.0 milestone Apr 2, 2020
@hollasch hollasch self-assigned this Apr 2, 2020
@hollasch
Copy link
Collaborator Author

hollasch commented Apr 3, 2020

I loved it so much we now have ten kids and twenty head of cattle.

hollasch added a commit that referenced this issue Apr 3, 2020
@hollasch
image_texture rewrite
06c6a5a 
- Old constructor took image data plus dimensions, new constructor just
  takes the image filename and does all loading.

- Added error message when the image file is not found.

- Fixed numerous pixel lookup off-by-a-smidge bugs.

- Increased bullet-proofing of pixel lookup.

- Optimized code to avoid recalculating expressions many times.

- Simplified the calling sites.

- STB image load now specifies exactly three components per pixel (RGB)

- Added explicit member initialization for the default constructor. I'd
  love to delete the default constructor altogether, but this triggers
  warnings on some build environments.

Resolves #434
@hollasch hollasch mentioned this issue Apr 3, 2020
Merged
@hollasch hollasch closed this as completed Apr 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@hollasch hollasch

Labels
area: book area: code
Projects
None yet
Milestone
v3.1.0
Development

No branches or pull requests
1 participant
@hollasch