mwpowellhtx Posted April 27, 2016 Share Posted April 27, 2016 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? Quote Link to comment Share on other sites More sharing options...
GameMonetize Posted April 27, 2016 Share Posted April 27, 2016 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. adam 1 Quote Link to comment Share on other sites More sharing options...
mwpowellhtx Posted April 27, 2016 Author Share Posted April 27, 2016 @Deltakosh Well, I'd rather document a sane options, wouldn't you? Quote Link to comment Share on other sites More sharing options...
mwpowellhtx Posted April 27, 2016 Author Share Posted April 27, 2016 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; } } } Quote Link to comment Share on other sites More sharing options...
GameMonetize Posted April 28, 2016 Share Posted April 28, 2016 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); Quote Link to comment Share on other sites More sharing options...
mwpowellhtx Posted April 28, 2016 Author Share Posted April 28, 2016 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. Quote Link to comment Share on other sites More sharing options...
GameMonetize Posted April 28, 2016 Share Posted April 28, 2016 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. Quote Link to comment Share on other sites More sharing options...
mwpowellhtx Posted April 28, 2016 Author Share Posted April 28, 2016 Where is there a drawString? Especially when context has the textAlign? You aren't making any sense. Quote Link to comment Share on other sites More sharing options...
mwpowellhtx Posted April 28, 2016 Author Share Posted April 28, 2016 The other thing is, no one is paying me for this. So be glad you have a potential open source contributor. Quote Link to comment Share on other sites More sharing options...
GameMonetize Posted April 28, 2016 Share Posted April 28, 2016 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. adam, Dad72, jerome and 1 other 4 Quote Link to comment Share on other sites More sharing options...
mwpowellhtx Posted April 28, 2016 Author Share Posted April 28, 2016 @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. Quote Link to comment Share on other sites More sharing options...
mwpowellhtx Posted April 28, 2016 Author Share Posted April 28, 2016 Have a look at the PR please. I'll look at the docs afterwards. Merged with the latest updates as well. Quote Link to comment Share on other sites More sharing options...
RaananW Posted April 28, 2016 Share Posted April 28, 2016 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, jerome and GameMonetize 3 Quote Link to comment Share on other sites More sharing options...
adam Posted April 28, 2016 Share Posted April 28, 2016 9 hours ago, Deltakosh said: I will just ignore you starting right now. I wish there was an option for this in the forum. GameMonetize, RaananW and Boz 3 Quote Link to comment Share on other sites More sharing options...
Dad72 Posted April 28, 2016 Share Posted April 28, 2016 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. . . GameMonetize 1 Quote Link to comment Share on other sites More sharing options...
jerome Posted April 28, 2016 Share Posted April 28, 2016 @adam : actually this feature exists on this forum :-) check your account settings => ignored users I just enabled it GameMonetize and adam 2 Quote Link to comment Share on other sites More sharing options...
mwpowellhtx Posted April 28, 2016 Author Share Posted April 28, 2016 @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. Quote Link to comment Share on other sites More sharing options...
Boz Posted April 28, 2016 Share Posted April 28, 2016 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 Samuel Girardin, GameMonetize, jerome and 1 other 4 Quote Link to comment Share on other sites More sharing options...
Dad72 Posted April 28, 2016 Share Posted April 28, 2016 3 hours ago, mwpowellhtx said: Alright, enough time wasted over nonsense. You are right, your time could be better used. Quote Link to comment Share on other sites More sharing options...
jerome Posted April 28, 2016 Share Posted April 28, 2016 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 Quote Link to comment Share on other sites More sharing options...
mwpowellhtx Posted April 28, 2016 Author Share Posted April 28, 2016 @jerome just went on my ignore list 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.