Wrong variable in uvs in VertexData.CreateGround?

[JohnK]

Never very confident in these things but in BJS2.5 shouldn't line 33527

uvs.push(col / subdivisionsX, 1.0 - row / subdivisionsX);

in the VertexData.CreateGround function below be

uvs.push(col / subdivisionsX, 1.0 - row / subdivisionsY);

Fairly minor as I expect most grounds will be square. Then again the code may be correct and I have misunderstood something.

VertexData.CreateGround = function (options) {
            var indices = [];
            var positions = [];
            var normals = [];
            var uvs = [];
            var row, col;
            var width = options.width || 1;
            var height = options.height || 1;
            var subdivisionsX = options.subdivisionsX || options.subdivisions || 1;
            var subdivisionsY = options.subdivisionsY || options.subdivisions || 1;
            for (row = 0; row <= subdivisionsY; row++) {
                for (col = 0; col <= subdivisionsX; col++) {
                    var position = new BABYLON.Vector3((col * width) / subdivisionsX - (width / 2.0), 0, ((subdivisionsY - row) * height) / subdivisionsY - (height / 2.0));
                    var normal = new BABYLON.Vector3(0, 1.0, 0);
                    positions.push(position.x, position.y, position.z);
                    normals.push(normal.x, normal.y, normal.z);
                    uvs.push(col / subdivisionsX, 1.0 - row / subdivisionsX);
                }
            }

 

[adam]

The subdivisionsX and subdivisionsY options is so that you can create ground that is not square.

edit: not square and control the spacing of subdivisions. 

[adam]

http://www.babylonjs-playground.com/#1WIONF

http://www.babylonjs-playground.com/#WJPB9#28

[JohnK]

@adam yes I understand the reasons for subdivisionsX and subdivisionsY my contention is that dividing the row by subdivisionsX rather than subdivisionsY you will get the wrong uv values in the z-direction. I am suggesting that a correction is made to the line I state.

 

[adam]

Good find.  Maybe that is an issue.

[adam]

I'm thinking it isn't an issue.

http://www.babylonjs-playground.com/#WJPB9#29

http://www.babylonjs-playground.com/#WJPB9#32

[JohnK]

Usually you cover the ground with some nondescript texture. If for some reason you wanted all of the image over the ground then the current code does not allow that.

http://www.babylonjs-playground.com/#1WIONF#1

However I can see that if you use a typical ground type image then the current method does not stretch the image in the z direction ( I think) so perhaps that the reason for dividing by subdivisionsX rather than subdivisionsY.

[adam]

It's coming back to me.  I think when I added the subdivisionsX and subdivisionsY feature, I added subdivisionsY and then simply changed the subdivisions var to subdivisionsX.  

[adam]

https://github.com/BabylonJS/Babylon.js/commit/1be2abfb505d84e70cd009484e2440cc5c8a1ed6

[JohnK]

So was the change in the line I mentioned from Y to X an on purpose one or an accidental one?  Hadn't realised you were responsible for the change - good contribution to BabylonJS.

[adam]

Well, all the subdivisions vars were updated to subdivisionsX.  So I guess it was on purpose.  

https://github.com/BabylonJS/Babylon.js/commit/1be2abfb505d84e70cd009484e2440cc5c8a1ed6#diff-a1b01c7e770753c86b95dc39689eb101L1201

That worked for me, so I left that way (I didn't want any stretching of the texture).

18 minutes ago, JohnK said:

good contribution to BabylonJS.

Thanks

edit: I should have said all the subdivisions vars were changed to subdivisionsX that were not required to be changed to subdivisionsY for the new feature. (I've written a lot of code since then)