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

Unbind integer textures #7273

Merged
merged 7 commits into from
Mar 15, 2021
Merged

Unbind integer textures #7273

merged 7 commits into from
Mar 15, 2021

Conversation

ivanpopelyshev
Copy link
Collaborator

@ivanpopelyshev ivanpopelyshev commented Mar 4, 2021

All sample types in shaders can have u or i prefix: usampler2D, isamplerCube

I added those types in SAMPLER_TYPES. To ensure that batcher doesn't use sampler of wrong type, and drawcall doesnt crash, TextureSystem unbinds those kind of textures.

Problem: https://www.html5gamedevs.com/topic/46983-mesh-graphics-render-issue/
Spawns error in console: https://www.pixiplayground.com/#/edit/xpLx-ElnpPPPXFJUkxoRa
Doesnt spawn errors: https://www.pixiplayground.com/#/edit/xpLx-ElnpPPPXFJUkxoRa

This PR conflicts with #7222 in afterEach thingy

@codecov
Copy link

codecov bot commented Mar 4, 2021

Codecov Report

Merging #7273 (13e5a11) into dev (077cab3) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##               dev     #7273   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           17        17           
  Lines          699       699           
=========================================
  Hits           699       699           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 077cab3...13e5a11. Read the comment docs.

Copy link
Member

@bigtimebuddy bigtimebuddy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll defer to someone else (@GoodBoyDigital) for the functionality here, but code quality looks fine. Just a few minor comments.

And welcome back @ivanpopelyshev glad to see PRs from you again.

packages/core/src/textures/GLTexture.ts Outdated Show resolved Hide resolved
packages/core/src/textures/TextureSystem.ts Outdated Show resolved Hide resolved
Copy link
Member

@ShukantPal ShukantPal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good addition! We should add it to PixiJS Examples.

Copy link
Member

@GoodBoyDigital GoodBoyDigital left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!

@bigtimebuddy bigtimebuddy added the ✅ Ready To Merge Helpful when issues are in the queue waiting to get merged. This means the PR is completed and has t label Mar 9, 2021
@bigtimebuddy bigtimebuddy added this to the v6.0.1 milestone Mar 15, 2021
@bigtimebuddy bigtimebuddy merged commit 5bad0d4 into dev Mar 15, 2021
@bigtimebuddy bigtimebuddy deleted the dev-ensure-sampler-type branch March 15, 2021 23:08
@blakjak44
Copy link

Hey guys, seems like something from @ivanpopelyshev's fix is missing. Using v6.0.2 I am still unable to render integer textures along with float textures.

v6.0.2: https://www.pixiplayground.com/#/edit/hMVY0qgOa6lLoNTi_PnEX <-- Not working
@ivanpopelyshev custom build: https://www.pixiplayground.com/#/edit/xpLx-ElnpPPPXFJUkxoRa <-- Works

@bigtimebuddy
Copy link
Member

That's my fault. Looks like it never actually got cherry-picked to 6.0.2. Will make sure it's 6.0.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✅ Ready To Merge Helpful when issues are in the queue waiting to get merged. This means the PR is completed and has t
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants