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
Do division and mod on unsigned integers #2496
Conversation
The hlsl compiler was complaining about doing division on signed integers being slow so we'll cast do our div and cast back. This fixes bug 1443747
@@ -100,7 +100,8 @@ void markCacheTexturesUsed() { | |||
// TODO: convert back to a function once the driver issues are resolved, if ever. | |||
// https://github.com/servo/webrender/pull/623 | |||
// https://github.com/servo/servo/issues/13953 | |||
#define get_fetch_uv(i, vpi) ivec2(vpi * (i % (WR_MAX_VERTEX_TEXTURE_WIDTH/vpi)), i / (WR_MAX_VERTEX_TEXTURE_WIDTH/vpi)) | |||
// Do the division with unsigned ints because that's more efficient with D3D | |||
#define get_fetch_uv(i, vpi) ivec2(int(uint(vpi) * (uint(i) % uint(WR_MAX_VERTEX_TEXTURE_WIDTH/vpi))), int(uint(i) / uint(WR_MAX_VERTEX_TEXTURE_WIDTH/vpi))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not certain this is correct. WR_MAX_VERTEX_TEXTURE_WIDTH
is defined as a simple integer constant, so WR_MAX_VERTEX_TEXTURE_WIDTH / vpi
is still integer division. Also, I don't think GLSL requires you to do uint(int(x), int(y))
as opposed to just uint(x, y)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WR_MAX_VERTEX_TEXTURE_WIDTH / vpi gets constant folded so using integer division there shouldn't matter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also uint(x, y) doesn't seem to work:
'constructor' : too many arguments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, thanks!
@@ -100,7 +100,8 @@ void markCacheTexturesUsed() { | |||
// TODO: convert back to a function once the driver issues are resolved, if ever. | |||
// https://github.com/servo/webrender/pull/623 | |||
// https://github.com/servo/servo/issues/13953 | |||
#define get_fetch_uv(i, vpi) ivec2(vpi * (i % (WR_MAX_VERTEX_TEXTURE_WIDTH/vpi)), i / (WR_MAX_VERTEX_TEXTURE_WIDTH/vpi)) | |||
// Do the division with unsigned ints because that's more efficient with D3D | |||
#define get_fetch_uv(i, vpi) ivec2(int(uint(vpi) * (uint(i) % uint(WR_MAX_VERTEX_TEXTURE_WIDTH/vpi))), int(uint(i) / uint(WR_MAX_VERTEX_TEXTURE_WIDTH/vpi))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, thanks!
@bors-servo r+ |
📌 Commit fa1090c has been approved by |
Do division and mod on unsigned integers The hlsl compiler was complaining about doing division on signed integers being slow so we'll cast do our div and cast back. This fixes bug 1443747 <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/2496) <!-- Reviewable:end -->
☀️ Test successful - status-appveyor, status-taskcluster, status-travis |
The hlsl compiler was complaining about doing division on signed
integers being slow so we'll cast do our div and cast back.
This fixes bug 1443747
This change is