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

SOIL_save_screenshot heap corruption #19

Closed
SpartanJ opened this issue Mar 13, 2018 · 6 comments
Closed

SOIL_save_screenshot heap corruption #19

SpartanJ opened this issue Mar 13, 2018 · 6 comments
Labels
bug Something isn't working major

Comments

@SpartanJ
Copy link
Owner

Original report by Anonymous.


Hi,

I've found the glReadPixels call in the SOIL_save_screenshot function occasionally corrupts the heap (depending on odd screen dimensions).

I believe its due to the GL_PACK_ALIGNMENT not being set. My fix is to surround the glReadPixels call with the following:

GLuint nOrgAlignment = 0;
glGetIntegerv(GL_PACK_ALIGNMENT, &nOrgAlignment);

// Ensure proper alignment
if(nOrgAlignment != 1)
	glPixelStorei(GL_PACK_ALIGNMENT, 1);

glReadPixels (x, y, width, height, GL_RGB, GL_UNSIGNED_BYTE, pixel_data);

// Restore original alignment
if (nOrgAlignment != 1)
	glPixelStorei(GL_PACK_ALIGNMENT, nOrgAlignment);
@SpartanJ
Copy link
Owner Author

Original comment by Martín Lucas Golini (Bitbucket: SpartanJ, GitHub: SpartanJ).


Sounds you're right! I'll take a look at this after work.

Thanks for reporting it!

@SpartanJ
Copy link
Owner Author

Original comment by Terry Russell (Bitbucket: xsensordev, ).


Certainly, no problem.

@SpartanJ
Copy link
Owner Author

Original comment by Martín Lucas Golini (Bitbucket: SpartanJ, GitHub: SpartanJ).


Fixed issue #19. Thanks Terry Russell!

@SpartanJ
Copy link
Owner Author

Original comment by Terry Russell (Bitbucket: xsensordev, ).


Glad to help. Though I notice you've selected the GL_UNPACK_ALIGNMENT instead of the GL_PACK_ALIGNMENT. Changing the GL_PACK_ALIGNMENT to 1 (from 4) was definitely the change I needed to prevent a heap corruption. I tested setting only the GL_UNPACK_ALIGNMENT and was still corrupting the heap when my window width was an odd value due to user re-sizing (ie: 807x423). This was with the latest nvidia drivers on Windows 10.

Perhaps both should be set to 1?

-Terry

@SpartanJ
Copy link
Owner Author

Original comment by Martín Lucas Golini (Bitbucket: SpartanJ, GitHub: SpartanJ).


That's what happens when you try to do something while tired ( and didn't even tested ). It's PACK aligment. Sorry!

PD: Commited the real fix.

@SpartanJ
Copy link
Owner Author

Original comment by Terry Russell (Bitbucket: xsensordev, ).


Looks good. Thanks for maintaining this useful library!

@SpartanJ SpartanJ added major bug Something isn't working labels Jan 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working major
Projects
None yet
Development

No branches or pull requests

1 participant