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

Shaders: Move bicubic texture filtering to transmission chunk. #25563

Merged
merged 1 commit into from
Feb 27, 2023

Conversation

Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Feb 24, 2023

Related issue: #25483, https://discourse.threejs.org/t/potential-regression-v149-v150/48397

Description

r150 introduces a regression since the bicubic texture filtering was added to common.glsl. This chunk is used in all materials as well as vertex and fragment shaders. However:

  • common.glsl should not hold logic which is vertex or fragment shader specific.
  • common.glsl should not hold GLSL 3 specific code which breaks WebGL 1 (like textureSize or textureLod).

To be clear: The current code in common.glsl breaks all WebGL 1 apps. This change needs to be cherry-picked to master.

@epreston

This comment was marked as off-topic.

@LeviPesin

This comment was marked as off-topic.

@epreston

This comment was marked as off-topic.

@epreston

This comment was marked as off-topic.

@Mugen87

This comment was marked as off-topic.

paulmasson referenced this pull request in erichlof/THREE.js-PathTracing-Renderer Feb 26, 2023
@mrdoob mrdoob added this to the r150 milestone Feb 27, 2023
@mrdoob mrdoob merged commit de5d6bf into mrdoob:dev Feb 27, 2023
@mrdoob
Copy link
Owner

mrdoob commented Feb 27, 2023

To be clear: The current code in common.glsl breaks all WebGL 1 apps. This change needs to be cherry-picked to master.

Done! And published 0.150.1 on npm.

@paulmasson
Copy link
Contributor

@mrdoob are you planning to update this release on JSDelivr, as you have done in the past (notably r137). And I'd love to know how you are able to update static branches there, as you are clearly able to do. Thanks!

@rahulp-open
Copy link

I have a vite project where I am using threejs library. configured the project with vite latest.

After bulding prod build, Project size is 503KBs.

Same project built with v0.148.1 of threejs is 103KBs.

this is huge change in bundle size, what caused this issue!!!??

@epreston
Copy link
Contributor

epreston commented Mar 2, 2023

@rahulp-open few things:

  • writing out your issue here ensures not many people will see it
  • 148.1 does not exist
  • there's only a 2 KB difference in 148 and 150

@rahulp-open
Copy link

It's making a lot of changes in build size. Build size went from 100kb to 500kb.

@epreston
Copy link
Contributor

epreston commented Mar 2, 2023

You've disabled minification in vite. It's impossible for the library to be larger than 380 KB otherwise. Please raise a separate issue.

@rahulp-open
Copy link

rahulp-open commented Mar 2, 2023

This is V 1.150.0

image

This is V 0.148.0

image

@rahulp-open
Copy link

rahulp-open commented Mar 2, 2023

@epreston

You've disabled minification in vite. It's impossible for the library to be larger than 380 KB otherwise. Please raise a separate issue.

image

minify was set to esBuild for both threejs versions.

@epreston
Copy link
Contributor

epreston commented Mar 2, 2023

Raise a separate issue please.

@epreston
Copy link
Contributor

epreston commented Mar 2, 2023

You're last screenshot shows why you are having an issue building a library of a library using this tooling. Please tag me in the issue so I can point it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants