mathbox icon indicating copy to clipboard operation
mathbox copied to clipboard

RGBFormat removed from r137

Open divisuals opened this issue 3 years ago • 4 comments

Tried using recent THREE release (r149), and got the following error:

WARNING in ./node_modules/mathbox/build/esm/render/buffer/texture/datatexture.js 57:12-27 export 'RGBFormat' (imported as 'CONST') was not found in 'three' (possible exports: ACESFilmicToneMapping, ...

Per THREE r137, RGBFormat is removed:

Texture Remove RGBFormat. https://github.com/mrdoob/three.js/pull/23228, https://github.com/mrdoob/three.js/pull/23201, https://github.com/mrdoob/three.js/pull/23205, https://github.com/mrdoob/three.js/pull/23211, https://github.com/mrdoob/three.js/pull/23223, https://github.com/mrdoob/three.js/pull/23218, https://github.com/mrdoob/three.js/pull/23219, https://github.com/mrdoob/three.js/pull/23367 (@Mugen87)

As this is only used in 1 class, a simple fix could be to just replace it with RGBAFormat, per this recommendation.

divisuals avatar Mar 10 '23 14:03 divisuals

Not sure if this is relevant: CONST.RGBFormat => gl.RGB lookup was removed from util/three.js in this commit - 3987ae6 (Feb/17/2022).

Also, this error is not reproducible in any tests or examples. One has to import the mathbox esm module in a different downstream project to reproduce it.

divisuals avatar Mar 13 '23 04:03 divisuals

@divisuals Yeah, I looked into this a bit over the weekend.

Ideally, we'd (1) understand the intent of the original RGBFormat for 3-channel, and (2) preserve that with the current version of THREEJS.

But... I tried downgrading to r136 and couldn't get 3-channel colors to work (even after restoring the line from https://github.com/unconed/mathbox/commit/3987ae62b77e321171fd8862d1d838d85b71dc93).

I'm sort of inclined to just replace the current instance of RGBFormat with an explicit undefined, which would perfectly preserve the current behavior (including any associated bugs) and silence that warning.

@sritchie Does ☝️ sound reasonable to you?

ChristopherChudzicki avatar Mar 13 '23 13:03 ChristopherChudzicki

Haven't seen any movement on this, so sharing my short-term workaround:

  • As mathbox is directly managing the data textures, I could get around compilation errors by just changing RGBFormat to RGBAFormat on this line
  • The code compiles and doesn't create any obvious runtime errors

Changing gl params to RGBA throws expected error: texSubImage2D: ArrayBufferView not big enough for request.

@ChristopherChudzicki can you please share which 3-channel color example didn't work for you? Thanks

divisuals avatar May 03 '23 23:05 divisuals

Chris: I tried downgrading to r136 and couldn't get 3-channel colors to work (even after restoring the line from https://github.com/unconed/mathbox/commit/3987ae62b77e321171fd8862d1d838d85b71dc93). divisuals can you please share which 3-channel color example didn't work for you? Thanks

@divisuals I believe I was just trying a surface with colorExpr emitting 3-components instead of 4. But I'm not sure if that's what "3 channel color" was supposed to do.

Re my comment above:

I'm sort of inclined to just replace the current instance of RGBFormat with an explicit undefined, which would perfectly preserve the current behavior (including any associated bugs) and silence that warning.

I just opened a PR that does this #69; that branch is published on npm as [email protected] if you want to try it in your project. This change seems pretty safe, but I want to do a bit more manual testing before publishing it.

Thanks for poking us about this.

ChristopherChudzicki avatar May 05 '23 10:05 ChristopherChudzicki