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

Remove UV warning when setting textures with setUniform() #5512

Closed
1 of 17 tasks
aferriss opened this issue Dec 23, 2021 · 7 comments · Fixed by #6474
Closed
1 of 17 tasks

Remove UV warning when setting textures with setUniform() #5512

aferriss opened this issue Dec 23, 2021 · 7 comments · Fixed by #6474

Comments

@aferriss
Copy link
Contributor

Most appropriate sub-area of p5.js?

  • Accessibility (Web Accessibility)
  • Build tools and processes
  • Color
  • Core/Environment/Rendering
  • Data
  • DOM
  • Events
  • Friendly error system
  • Image
  • IO (Input/Output)
  • Localization
  • Math
  • Unit Testing
  • Typography
  • Utilities
  • WebGL
  • Other (specify if possible)

Details about the bug:

If you are using the vertex function and supplying your own UV's, when using the setUniform() function to send a texture to a custom shader, a warning is triggered telling you: "You must first call texture() before using vertex() with image based u and v coordinates ".

Since we're supplying the texture with setUniform(), we shouldn't be showing this warning.

Actually, I think there is a case to be made to remove this warning entirely. It could be that the user is writing a procedural shader, and not using textures, in which case, the warning isn't necessary at all.

The warning seems to go away when using textureMode(NORMAL);

  • p5.js version: 1.4
  • Web browser and version: Chrome 96
  • Operating System: Mac OSX
  • Steps to reproduce this:
  1. Load a custom shader that samples a texture
  2. Send the texture to the shader
  3. Draw some geometry with beginShape(), vertex() ....
  4. Observe the warning.

Here's a small example showing the issue

@Yash621
Copy link

Yash621 commented Feb 19, 2022

@aferriss can I work on this issue ?

@aferriss
Copy link
Contributor Author

@Yash621 Of course, go right ahead!

@Qianqianye Qianqianye added this to the 1.5.0 milestone Jul 27, 2022
@Gaurav-1306
Copy link
Contributor

Can the admins suggest that if removing this line with close the issue.

) {
// Only throw this warning if custom uv's have been provided
console.warn(
'You must first call texture() before using' +
' vertex() with image based u and v coordinates'

Or I can write a if statement that can check if setUniform has been called and then no warning is issured.

@Qianqianye
Copy link
Contributor

Thanks @Gaurav-1306. What do you think, @aferriss and @davepagurek?

@davepagurek
Copy link
Contributor

I think that line is indeed the one causing the error! I think we still want it, so like you said, we'd add to the if statement condition so that it doesn't get logged in as many cases.

I think we don't even need to check if setUniform has been called, since some shaders will use the texture coordinates without needing a uniform. So maybe we can just check if there's any user shader at all? e.g. if any of these is not undefined?

this.userFillShader = undefined;
this.userStrokeShader = undefined;
this.userPointShader = undefined;

@Gaurav-1306
Copy link
Contributor

Gaurav-1306 commented Oct 15, 2023

@davepagurek

  if (
    this.textureMode === constants.IMAGE &&
    !this.isProcessingVertices
  ) {
    if (this._tex !== null ) {
      if (this._tex.width > 0 && this._tex.height > 0 ) {
        u /= this._tex.width;
        v /= this._tex.height;
      }
    } else if (
      (this.userFillShader !== undefined ||
      this.userStrokeShader !== undefined ||
      this.userPointShader !== undefined)
    ) {
    // Do nothing if user-defined shaders are present
    } else if (
      this._tex === null &&
      arguments.length >= 4
    ) {
      // Only throw this warning if custom uv's have  been provided
      console.warn(
        'You must first call texture() before using' +
          ' vertex() with image based u and v coordinates'
      );
    }
  }

i came up with this implementation. do you think it is good to be merged?

i basically added a else if statement with the new conditions. i tested it using the lib/empty-example. it was showing no error.

@davepagurek
Copy link
Contributor

I think that looks good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: DONE! 🎉
Development

Successfully merging a pull request may close this issue.

5 participants