PeapBoy Posted July 2, 2018 Share Posted July 2, 2018 Hello ! As I'm currently playing with some unsigned integer textures, I would love to see BabylonJS support them. I'll do a PR as soon as possible, adding some (all ?) constants about internal formats (for instance Engine.TEXTUREFORMAT_RGBA_INTEGER) and texture types (for instance Engine.TEXTURETYPE_UNSIGNED_SHORT), and updating the function to retrieve the internal sized format in order to be able to return new values as gl.RGBA16UI. Does it seem ok for you ? Moreover, we're still using Engine.TEXTURETYPE_UNSIGNED_INT to refer to both gl.UNSIGNED_INT (only available for depth textures in WebGL 1) and gl.UNSIGNED_BYTE texture types. I think we need to do a breaking change because gl.UNSIGNED_INT is now a valid texture type for unsigned integer textures in WebGL2 like RGBA32UI textures. Valid combinations of internal format, type and internal size format are listed here in table 3.2. Color-renderable and texture-filterable formats are listed here in table 3.13. This info is also gathered here. And this is a bit less exhaustive in WebGL 2 Specs. [EDIT] With WebGL2, specify the internal sized format will be necessary as some combinations of format and type don't lead to a unique internal sized format. For this purpose, we'll need to add internal sized formats as constants in BABYLON.Engine and we won't lean on _getRGBABufferInternalSizedFormat() function anymore. However, only a few combinations fail to give a unique result: - RGBA format + UNSIGNED_BYTE type - RGBA format + UNSIGNED_INT_2_10_10_10_REV type - RGBA format + FLOAT type - RGB format + UNSIGNED_BYTE type - RGB format + HALF_FLOAT type - RGB format + FLOAT type - RG format + FLOAT type - RED format + FLOAT type So we could just ignore them and return a default value in these cases, for now. [EDIT 2] Current type constants: private static _TEXTURETYPE_UNSIGNED_INT = 0; private static _TEXTURETYPE_FLOAT = 1; private static _TEXTURETYPE_HALF_FLOAT = 2; Suggested new type constants: private static _TEXTURETYPE_BYTE = 0; private static _TEXTURETYPE_UNSIGNED_BYTE = 1; private static _TEXTURETYPE_SHORT = 2; private static _TEXTURETYPE_UNSIGNED_SHORT = 3; private static _TEXTURETYPE_INT = 4; private static _TEXTURETYPE_UNSIGNED_INT = 5; private static _TEXTURETYPE_FLOAT = 6; private static _TEXTURETYPE_HALF_FLOAT = 7; private static _TEXTURETYPE_UNSIGNED_SHORT_4_4_4_4 = 8; private static _TEXTURETYPE_UNSIGNED_SHORT_5_5_5_1 = 9; private static _TEXTURETYPE_UNSIGNED_SHORT_5_6_5 = 10; private static _TEXTURETYPE_UNSIGNED_INT_2_10_10_10_REV = 11; private static _TEXTURETYPE_UNSIGNED_INT_24_8 = 12; private static _TEXTURETYPE_UNSIGNED_INT_10F_11F_11F_REV = 13; private static _TEXTURETYPE_UNSIGNED_INT_5_9_9_9_REV = 14; private static _TEXTURETYPE_FLOAT_32_UNSIGNED_INT_24_8_REV = 15; NOTE 1: As every user should use the public constants and not directly the private numbers the constants are bound to, we should be allowed to modify the order (FLOAT will be defined by 6 instead of 1 for instance). If you do not want to change the order of the three first constants already defined, just say it. NOTE 2: UNSIGNED_INT will not refer to UNSIGNED_BYTE anymore. Breaking change. [EDIT 3] Current format constants: private static _TEXTUREFORMAT_ALPHA = 0; private static _TEXTUREFORMAT_LUMINANCE = 1; private static _TEXTUREFORMAT_LUMINANCE_ALPHA = 2; private static _TEXTUREFORMAT_RGB = 4; private static _TEXTUREFORMAT_RGBA = 5; private static _TEXTUREFORMAT_R = 6; private static _TEXTUREFORMAT_RG = 7; NOTE 1: No 3 ? Did I miss something ? NOTE 2 : I personally guess TEXTUREFORMAT_R is not a good idea for 3 reasons: - It's confusing for people already used to WebGL who know it's gl.RED and not gl.R - It doesn't seem really instructive for beginners who will think R is the good naming - Beginners generally don't play with texture formats and types That's why I suggest to create and use TEXTUREFORMAT_RED in the future. And keep TEXTUREFORMAT_R for retrocompatibility. But once again, it's only a suggestion. Suggested new format constants: private static _TEXTUREFORMAT_ALPHA = 0; private static _TEXTUREFORMAT_LUMINANCE = 1; private static _TEXTUREFORMAT_LUMINANCE_ALPHA = 2; private static _TEXTUREFORMAT_RED = 3; private static _TEXTUREFORMAT_R = 3; private static _TEXTUREFORMAT_RG = 4; private static _TEXTUREFORMAT_RGB = 5; private static _TEXTUREFORMAT_RGBA = 6; private static _TEXTUREFORMAT_RED_INTEGER = 7; private static _TEXTUREFORMAT_R_INTEGER = 7; private static _TEXTUREFORMAT_RG_INTEGER = 8; private static _TEXTUREFORMAT_RGB_INTEGER = 9; private static _TEXTUREFORMAT_RGBA_INTEGER = 10; NOTE 1: Once again, we should be allowed to change the order of the first constants as nobody should use the private numbers directly. brianzinn, NasimiAsl and dbawel 2 1 Quote Link to comment Share on other sites More sharing options...
Guest Posted July 2, 2018 Share Posted July 2, 2018 ok:) so first thank you for helping! then my remarks in not particular order: 1. We can't change the values of the constants because several users use the value directly. This is bad but they do want they want This is why there is no 3 in texture format. I missed it when I created the _RBG one and now we cannot change it anymore as backward compat is king 2. There is no way we introduce breaking changes 3. I do not want to expose internal format. We could add a validation step to make sure that combination works but I do not want to let people know about internals because else we won't be able to change anything then (yeah, because back compat is king :D) 4. Cannot change R to RED (yeah I know that you know why) And honestly this is not a big deal for this one because else why keeping RG? it should be REDGREEN 5. As far as I know, UNSIGNED_INT always refer to BYTE actually, I don't think it was ever used for something else for textures. for real integer we should come with a new name (and we can also introduce a BYTE which will have the same value as the current UNSIGNED_INT) Also please just way until Thursday for your PR because we are moving some constants just a bit (but nothing fancy and we are not changing values or names) dbawel 1 Quote Link to comment Share on other sites More sharing options...
PeapBoy Posted July 2, 2018 Author Share Posted July 2, 2018 Hi! 1. Noted. 2. So UNSIGNED_INT won't refer to UNSIGNED_INT and we'll invent UNSIGNED_INTEGER (or anything else) that actually will refer to UNSIGNED_INT, I really hate backwards compatibility ? 3. It's fine for me, the impacted combinations that will not be possible don't affect my needs and I think they're very unsual. 4. Because RG is the good naming. It's gl.RED, gl.RG, GL.RGB and gl.RGBA. It's absolutely not important though, just my will to add a constant with the good naming, I didn't plan to remove gl.R at all, just a new one (RED) referring to the same as R. 5. My bad, you're right, UNSIGNED_INT is only referring to UNSIGNED_BYTE, you directly call gl.UNSIGNED_INT for depth textures and drawElements() functions. So I suggest UNSIGNED_INTEGER as a new name for UNSIGNED_INT but I think this should be strongly stated in the docs. Ok, thank you, understood! Quote Link to comment Share on other sites More sharing options...
Vousk-prod. Posted July 2, 2018 Share Posted July 2, 2018 Just to add my two cents. Sometimes keeping too much backward compatibility lead to awful paterns, ugly code and less and less optimised libs... I think we should avoid BJS to become an IE kind of mess ? Breaking changes are not convenient for users, but this is exactly what SEMVER is for. This is the price to pay to keep a strong tool over time. We sometimes have to dare to take some painful decisions to ensure powerfull and long life to the lib. Users can understand, accept and adapt. It is for their final good. "Simple, yet powerfull" does not mean "so simple you never have to adapt", it means "to use it you won't have to do lots of complicated coding weirdness". And "powerfull" means true direct power, not a degraded power due to twisted backward compatibility. My opinion ? PeapBoy and dbawel 1 1 Quote Link to comment Share on other sites More sharing options...
Guest Posted July 3, 2018 Share Posted July 3, 2018 I agree on that but this specific one (UNSIGNED_INT) has no impact on performance or code patterns. It is just a naming problem. And I would be really OK to change it (like R -> RED seems ok for me as we can provide both) if it was not that widely used. We can't change it as ALL textures use it as a default. So every app written in typescript will no more compile. every app written in JS with a reference to the constant will not compile. I am really concerned by perf and framework quality. And sometimes I validate breaking changes (just check the what's new here: http://doc.babylonjs.com/whats-new) But in this specific case, just for a naming error (that I did), we cannot afford breaking all the code of all the users. That's not good for the framework Quote Link to comment Share on other sites More sharing options...
Nabroski Posted July 3, 2018 Share Posted July 3, 2018 phu. a lot to read as i interpret Deltakosh @PeapBoy you can do what ever you want as long you end up with something like new BABYLON.WebGL2Texture("PATH TO IMAGE", scene); and it is progressive, performant also don't consolidate with the existing implementation. PeapBoy 1 Quote Link to comment Share on other sites More sharing options...
PeapBoy Posted July 4, 2018 Author Share Posted July 4, 2018 Uh. Of course they exist ? Sorry but what should I do with gulp so it doesn't fail compiling because of the new WebGL2 constants ? I saw this in the code: // Constants this._gl.HALF_FLOAT_OES = 0x8D61; // Half floating-point type (16-bit). if (this._gl.RGBA16F !== 0x881A) { this._gl.RGBA16F = 0x881A; // RGBA 16-bit floating-point color-renderable internal sized format. } if (this._gl.RGBA32F !== 0x8814) { this._gl.RGBA32F = 0x8814; // RGBA 32-bit floating-point color-renderable internal sized format. } if (this._gl.DEPTH24_STENCIL8 !== 35056) { this._gl.DEPTH24_STENCIL8 = 35056; } How is it possible that we need to rewrite some constants !? Nabroski 1 Quote Link to comment Share on other sites More sharing options...
Nabroski Posted July 4, 2018 Share Posted July 4, 2018 Hello thank you very much in advance for all your work! I read in 2018 most mobiles don't even support RGB32F as example, i found this just now and don't know accurate this is: https://gist.github.com/TimvanScherpenzeel/f8efeeb1dbed38a5c5dc0c29768a0413 So lets say you implement DEPTH_COMPONENT16, you also have to make it work in the source of BabylonJS specially shaders. Just think about it, maybe it make more sense to hook up in BabylonJS in a very minimal state of the abstraction Layer to WebGL, so Babylonjs could adapt the new functions (when needed) slowly. https://doc.babylonjs.com/api/classes/babylon.nullengine Let's rephrase it for better understanding: Write raw WebGL or minimal BabylonJS and then at the end "bind" the hole thing in some BabylonJS like method: function=()=>{ gl.getContext("WebGL2"); if (gl && gl instanceof WebGLRenderingContext) let Object={ texturetypes: "ALLWEBGL2TextureTypes" }, BABYLON._gl = gl.texImage2D(gl.TEXTURE_2D, level, internalFormat, width, height, border, format, Object.texturetypes, data); return new BABYLON.WebGL2Texture(ect.,ect.); } hahah. Sounds crazy, i don't even know what i doing here, should what a movie on Netflix. : ) Quote Link to comment Share on other sites More sharing options...
PeapBoy Posted July 5, 2018 Author Share Posted July 5, 2018 Hi ! Actually it's not a lot of work of adding a few constants but thanks haha 12 hours ago, Nabroski said: I read in 2018 most mobiles don't even support RGB32F as example, i found this just now and don't know accurate this is: https://gist.github.com/TimvanScherpenzeel/f8efeeb1dbed38a5c5dc0c29768a0413 Yes, principally Apple devices still don't support WebGL2 (so annoying...). But in such cases, BabylonJS already falls back on UNSIGNED_BYTE texture type. I admit I don't really get what you're trying to explain x) I don't really like the WebGL2Texture. I don't think the user should manage whether the devices support or not WebGL2, BabylonJS should do it as it already does, inside the Texture classes that already exist. Because it's one of the annoying task I expect Babylonjs to do for me. And I just discovered that BabylonJS provides RawTexture and RawCubeTexture that exactly do what you seem to expect from WebGL2Texture ? Adding the constants. Nothing changes. Every function/constructor that already wait for a format and a type parameter should work as usual. Nabroski 1 Quote Link to comment Share on other sites More sharing options...
Nabroski Posted July 5, 2018 Share Posted July 5, 2018 So you were close for a new release, i think, i should no interfear. yes something like this would be very nice. var myTex = BABYLON.RawTexture(new Float32Array([]),1,1, gl.RGB32F, true,false,0,null, gl.FLOAT); to compensate gl.texImage2D( gl.TEXTURE_2D, 0, gl.RGB32F, 9,1, 0, gl.RGB, gl.FLOAT,new Float32Array([])); Thank you! Quote Link to comment Share on other sites More sharing options...
PeapBoy Posted July 5, 2018 Author Share Posted July 5, 2018 Yes, just adding the constants and updating the _getRGBABufferInternalSizedFormat(), _getInternalFormat() and _getWebGLTextureType() functions should definitely be enough to create any texture you want with BABYLON.RawTexture or BABYLON.RawCubeTexture ! ? Quote Link to comment Share on other sites More sharing options...
PeapBoy Posted July 5, 2018 Author Share Posted July 5, 2018 Well, I succeeded in compiling the code. I needed to add all the WebGL2 constants in babylon.webgl2.ts. There is still a lot I don't know about how works BJS ? dbawel and GameMonetize 1 1 Quote Link to comment Share on other sites More sharing options...
PeapBoy Posted July 26, 2018 Author Share Posted July 26, 2018 Hi, Sorry, in holidays ? You can find the PR here: https://github.com/BabylonJS/Babylon.js/pull/4839 Nabroski 1 Quote Link to comment Share on other sites More sharing options...
Nabroski Posted July 26, 2018 Share Posted July 26, 2018 @PeapBoy texSubImage2D would be nice. but don't feel any pressure. I m fine, - as long as you know what you doing https://www.babylonjs-playground.com/#0GT2HX#3 Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.
Note: Your post will require moderator approval before it will be visible.