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

Cubemap renderTexture support #6435

Merged
merged 11 commits into from
Jun 18, 2020
Merged

Cubemap renderTexture support #6435

merged 11 commits into from
Jun 18, 2020

Conversation

ivanpopelyshev
Copy link
Collaborator

@ivanpopelyshev ivanpopelyshev commented Feb 26, 2020

  1. refactoring - move everything out ArrayResource to parent abstract class
  2. handle baseTexture binds differently in ArrayResource/CubeResource
  3. check for parentTextureArray when we bind a texture or framebuffer to texture.

Should help @GoodBoyDigital for his cases.

UPD. It works: https://www.pixiplayground.com/#/edit/4RPGjVMWfcWGyxHusdcKE

@GoodBoyDigital
Copy link
Member

oh! excite! will test...

@codecov-io
Copy link

codecov-io commented Feb 26, 2020

Codecov Report

Merging #6435 into dev will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##              dev    #6435   +/-   ##
=======================================
  Coverage   78.33%   78.33%           
=======================================
  Files          57       57           
  Lines        2820     2820           
=======================================
  Hits         2209     2209           
  Misses        611      611

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 67ff508...e924694. Read the comment docs.

@bigtimebuddy bigtimebuddy added 👀 Needs Review PR needs to be reviewed by someone on the core team. and removed Difficulty: Medium labels Mar 31, 2020
@bigtimebuddy
Copy link
Member

bigtimebuddy commented Jun 4, 2020

I can live with this, but I'm not a huge fan of introducing another abstraction layer. I already feel classes like BaseImageResource overly complicate an otherwise simple resource system and this adds yet another layer. Is there an easier way to achieve the same thing on the resources-side? Either with some static util functions, or having CubeResource override ArrayResource methods?

Also, this needed to resolve conflicts with dev.

@GoodBoyDigital
Copy link
Member

Hey this is cool! Thanks @ivanpopelyshev. We are going to roll this is as it adds a new useful feature 👍

We discussed and do feel that the resource manager has grown in complexity over time and would definitely benefit from a bit of a refactor, potentially using composition rather than inheritance!

watch this space!

@bigtimebuddy bigtimebuddy added ✅ Ready To Merge Helpful when issues are in the queue waiting to get merged. This means the PR is completed and has t and removed 👀 Needs Review PR needs to be reviewed by someone on the core team. labels Jun 17, 2020
@codecov-commenter
Copy link

Codecov Report

Merging #6435 into dev will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##               dev     #6435   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           15        15           
  Lines          671       671           
=========================================
  Hits           671       671           

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 dea7118...4d808ee. Read the comment docs.

@bigtimebuddy bigtimebuddy added the ⏰ Future Work Reminder for ourselves to flag work that doesn't make sense to do right now. label Jun 17, 2020
@bigtimebuddy bigtimebuddy added this to the v5.3.0 milestone Jun 17, 2020
@bigtimebuddy bigtimebuddy merged commit 140ca83 into dev Jun 18, 2020
@bigtimebuddy bigtimebuddy deleted the feature-rendertexture-cubemap branch June 18, 2020 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⏰ Future Work Reminder for ourselves to flag work that doesn't make sense to do right now. ✅ 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