DynamicTexture options is really confused

[mwpowellhtx]

IMO it is really confused. Options cannot make up its mind what it wants to be; being kind to the designers:

• A _canvas
• An object descriptor: i.e. { width: x, height: y }
• A size: i.e. { width: options, height: options }

Why do I say this?

I suggest making up its mind: be a descriptor:

if (options.canvas !== undefined) {
    //...
}

Or:

if (!(options.width === undefined || options.height === undefined)) {
    // ...
}
else if (options.size !== undefined) {
   // ...
}

This will cause some breaking changes, but I think it is acceptable to correct the course onto a path of sanity.

Next, I want to add text alignment

if (options.textAlign !== undefined && typeof options.textAlign === "string") {
   // ...
}

I'm glad to submit the PR. I just don't want for it to sit on the shelf when I do.

I would submit a Github issue, but it seems that has been disabled, or is unavailable to BabylonJS.

Is that agreeable?

[GameMonetize]

I don't get the difference for developers. They will still have to read the doc to know what options is, correct?

So I would prefer updating the doc to be clearer.

[mwpowellhtx]

@Deltakosh Well, I'd rather document a sane options, wouldn't you?

[mwpowellhtx]

Do I also need to touch the JavaScript file? Or is JavaScript parsed out of the TypeScript?

Proposed changes:

module BABYLON {
    export class DynamicTexture extends Texture {
        private _generateMipMaps: boolean;
        private _canvas: HTMLCanvasElement;
        private _context: CanvasRenderingContext2D;
        private _options: any;

        constructor(name: string, options: any, scene: Scene, generateMipMaps: boolean, samplingMode: number = Texture.TRILINEAR_SAMPLINGMODE) {
            super(null, scene, !generateMipMaps);

            this.name = name;
            this._options = options;

            this.wrapU = Texture.CLAMP_ADDRESSMODE;
            this.wrapV = Texture.CLAMP_ADDRESSMODE;

            this._generateMipMaps = generateMipMaps;

            /* there are probably clearer ways to verify all of this */
            if (!(options.canvas === undefined || options.canvas.getContext === undefined) && typeof options.canvas.getContext === "function") {
                this._canvas = options;
                this._texture = scene.getEngine().createDynamicTexture(options.canvas.width, options.canvas.height, generateMipMaps, samplingMode);
            } else {
                this._canvas = document.createElement("canvas");

                if (!(options.width === undefined || options.height === undefined)) {
                    this._texture = scene.getEngine().createDynamicTexture(options.width, options.height, generateMipMaps, samplingMode);
                } else if (options.size !=== undefined) {
                    this._texture = scene.getEngine().createDynamicTexture(options.size, options.size, generateMipMaps, samplingMode);
                }
            }

            var textureSize = this.getSize();

            this._canvas.width = textureSize.width;
            this._canvas.height = textureSize.height;
            this._context = this._canvas.getContext("2d");
        }

        public get canRescale(): boolean {
            return true;
        }

        public scale(ratio: number): void {
            var textureSize = this.getSize();

            textureSize.width *= ratio;
            textureSize.height *= ratio;

            this._canvas.width = textureSize.width;
            this._canvas.height = textureSize.height;

            this.releaseInternalTexture();

            this._texture = this.getScene().getEngine().createDynamicTexture(textureSize.width, textureSize.height, this._generateMipMaps, this._samplingMode);
        }

        public getContext(): CanvasRenderingContext2D {
            return this._context;
        }

        public clear(): void {
            var size = this.getSize();
            this._context.fillRect(0, 0, size.width, size.height);
        }

        public update(invertY?: boolean): void {
            this.getScene().getEngine().updateDynamicTexture(this._texture, this._canvas, invertY === undefined ? true : invertY);
        }

        public drawText(text: string, x: number, y: number, font: string, color: string, clearColor: string, invertY?: boolean, update = true) {
            var size = this.getSize();

            if (this._options.textAlign !== undefined && typeof this._options.textAlign === "string") {
                this._context.textAlign = this._options.textAlign;
            }

            if (clearColor) {
                this._context.fillStyle = clearColor;
                this._context.fillRect(0, 0, size.width, size.height);
            }

            this._context.font = font;
            if (x === null) {
                var textSize = this._context.measureText(text);
                x = (size.width - textSize.width) / 2;
            }

            this._context.fillStyle = color;
            this._context.fillText(text, x, y);

            if (update) {
                this.update(invertY);
            }
        }

        public clone(): DynamicTexture {
            var textureSize = this.getSize();
            var newTexture = new DynamicTexture(this.name, textureSize.width, this.getScene(), this._generateMipMaps);

            // Base texture
            newTexture.hasAlpha = this.hasAlpha;
            newTexture.level = this.level;

            // Dynamic Texture
            newTexture.wrapU = this.wrapU;
            newTexture.wrapV = this.wrapV;

            return newTexture;
        }
    }
} 

 

[GameMonetize]

I don't get why your options is saner.

 

What we want is simplicity for users, so they can write:

var texture = new BABYLON.DynamicTexture("foo", canvas, scene);

Or:

var texture = new BABYLON.DynamicTexture("foo", 512, scene);

or

var texture = new BABYLON.DynamicTexture("foo", {width:512, height: 1024}, scene);

 

[mwpowellhtx]

Or:

var texture = new BABYLON.DynamicTexture("foo", {width: 512, height: 1024, textAlign: "center"}, scene);

How it is "simpler" is that is more consistent.

I get how JavaScript is so weakly typed. That's great as far as it goes. But it can also get you into trouble. I don't have to scan much further than the first couple of if/else checks in the current version to see that.

I at least want the textAlign to be part of the options. It might also be nice to size-to-fit, if it were possible, of the container geometry, would make for more predictable dimensions, positioning, etc, at least for my application.

[GameMonetize]

I won't change the signature for sure. And I'm convinced that we must update the doc

Regarding textAlign, this is more a property of the drawString function to me.

[mwpowellhtx]

Where is there a drawString? Especially when context has the textAlign? You aren't making any sense.

[mwpowellhtx]

The other thing is, no one is paying me for this. So be glad you have a potential open source contributor.

[GameMonetize]

I meant: drawText (but don't be more stupid than you are, you've got my point).

The drawText could be improved to take in account more parameters. Or even better: we can have a drawTextEnhanced where we can find a options arguments.

 

For the record: no one is paying me for babylon.js at all. It is a spare time project. I'm working on Babylon.js because 3D is my passion. 

If you don't want to contribute, this is your call. Right now, you've reached my maximum acceptance factor for arrogance. I will just ignore you starting right now.

But perhaps others members of this forum will be more inclined than me to be insulted while trying to help you.

[mwpowellhtx]

@Deltakosh I get it. So I pitched a modified version. Yes or no on the options descriptor consistency? textAlign? Would be much better than having to work around the context apart from the interface.

[mwpowellhtx]

Have a look at the PR please. I'll look at the docs afterwards. Merged with the latest updates as well.

[RaananW]

Something I really want to say here - 

I am very glad you started your first post with IMO. Everyone has an opinion  

Yes, there might be a "saner" way of making everything, but eventually someone needs to decide on conformity, and this "someone" is the one that keeps on answering. Suggesting a change is not the same as enforcing a change. Knowing Deltakosh, if you suggest something and it makes sense, he will never say no. Whatever simpler will go in. and this is the reason why people love working with this framework. So even if I sometimes don't agree on things, I still understand that the fact that the bigger picture makes sense means SOMETHING - the vision is right.

And just one last little thing - I don't mean to sound disrespectful, but saying something like "you aren't making any sense" has no place in this forum (or any other forum, IMO). There is a thin line between disagreeing (or stating your opinion) and just being plain rude.

 

[adam]

9 hours ago, Deltakosh said:

I will just ignore you starting right now.

I wish there was an option for this in the forum.

[Dad72]

10 hours ago, mwpowellhtx said:

So be glad you have a potential open source contributor.

There are already a lot of contributor and we are very happy. I hallucinating when I read this, that claim/arrogance. . .

[jerome]

@adam : actually this feature exists on this forum :-)

check your account settings => ignored users

I just enabled it 

[mwpowellhtx]

@RaananW No disrespect intended, but of course it has a place. I asked for clarification. Take it how you will.

@adam @jerome No one is asking you to pay attention. Ignoring runs both ways, though I'd hate to lose you as a peer.

Alright, enough time wasted over nonsense.

[Boz]

I suggest you to try understanding how this framework works before saying bullshit.

All babylonJS contributors do it for free, and spend so much time on it, in addition to helping others via this forum. Of course disrespect was intended, hope it will change in the future

[Dad72]

3 hours ago, mwpowellhtx said:

Alright, enough time wasted over nonsense.

You are right, your time could be better used.

[jerome]

funny... since I use the "ignore list", I can just read the quoted sentences in the posts from the other users

really weird : you, guys, are having a discussion but I can only read a side of it 

that said... this smells like fresh air to avoid those bad feelings or disrespectful chats

[mwpowellhtx]

@jerome just went on my ignore list