Sign in to follow this  
mwpowellhtx

DynamicTexture options is really confused

Recommended Posts

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?

Share this post


Link to post
Share on other sites

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;
        }
    }
} 

 

Share this post


Link to post
Share on other sites

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);

 

Share this post


Link to post
Share on other sites

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.

Share this post


Link to post
Share on other sites

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.

Share this post


Link to post
Share on other sites

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.

 

Share this post


Link to post
Share on other sites
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. . .

Share this post


Link to post
Share on other sites

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

Share this post


Link to post
Share on other sites

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 :D

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

Share this post


Link to post
Share on other sites

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.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

Loading...
Sign in to follow this  

  • Recently Browsing   0 members

    No registered users viewing this page.