Skip to content
This repository has been archived by the owner on Jun 28, 2021. It is now read-only.

Support for Updating Rendering Options #34

Closed
ArronFerguson opened this issue Jan 4, 2017 · 9 comments
Closed

Support for Updating Rendering Options #34

ArronFerguson opened this issue Jan 4, 2017 · 9 comments

Comments

@ArronFerguson
Copy link

Not an issue but a request for a feature really.

If a developer wants to support updating rendering options (e.g., change font size) for when the web browser is resized by the end-user, the developer has to remove the text component from the PIXI container and re-add it. If there are many MultiStyleText objects on the screen, this can increase the code base and development time. The performance of doing a resize can also be affected as well.

So after the user creates the text object and adds it to a PIXI container:

var text = new MultiStyleText("<first>John</first> <last>Smith</last>",
{
    default: { fontFamily: "sf_slapstick_comicregular",
               fontSize: fontSize + "px"},
    first: { fill: "#e55c00"},
    last: { fill: "#4fc1b7" }
});
// ...
stage.addChild(topBarTextL);

It would be practical to later be able to say (for example):

fontSize += 10;

text.setStyleKey("default", {fontFamily: "sf_slapstick_comicregular",
    fontSize: fontSize + "px"});

This would keep all other style keys still intact with only changes to the current style.

One other thought ...

Don't know how much extra work this would be but if the properties within each style key are set to dictionaries already (so a dictionary of dictionaries), a developer could simply override whichever interested property/properties they wish to choose:

fontSize += 10;

text.setStyleKey("default", {  fontSize: fontSize + "px"});

This would only override the font size in the default style key.

@themoonrat
Copy link

If this lib adopted the use of PIXI.TextStyle lib, then I think most of your requests are covered. You see, you can create one PIXI.TextStyle instance, then give that to multiple PIXI.Text classes. Then, when you change the style, it emits an event that updates all of the text instances.

In this libs case, 'default' would be a TextStyle, and any [keys] would also be a TextStyle

@ArronFerguson
Copy link
Author

ArronFerguson commented Jan 4, 2017 via email

@bluepichu
Copy link
Collaborator

This is something I've realized and considered in the past. This is messy mostly because the only style that's necessarily a full TextStyle is default, which is necessary for nesting inheritance: if every tag has a full set of styles defined, then you couldn't do something like <b>Bold only</b> <i>Italic only</i> <b><i>Bold and Italic</i></b> and have it work.

That said, we could do something similar to PIXI.TextStyle and have getters and setters on every field - however, this doesn't cover the case of adding a new style, which can't be handled by a getter or setter because it can occur on any key. The only way to capture this behavior would be to use Proxy, which can't be polyfilled and thus would require us to either retarget for ES6 or use ES5 only with the exception of Proxy. (That said - current versions of Edge, Firefox, Chrome, Safari, Opera, and the iOS browser have full support for Proxy.)

@bluepichu
Copy link
Collaborator

bluepichu commented Jan 4, 2017

Looking again, I think I might've partially misidentified what you're trying to do.

Note that you can update a MultiStyleText's styles without removing it - the last example in the demo actually does this. You just need to reassign styles, which serves to both update the styles object and mark the text as dirty (so it gets rerendered). So, in your example:

let textStyles =   {
    default: { fontFamily: "sf_slapstick_comicregular", fontSize: fontSize + "px"},
    first: { fill: "#e55c00"},
    last: { fill: "#4fc1b7" }
};

let text = new MultiStyleText("<first>John</first> <last>Smith</last>", textStyles);

// ...

stage.addChild(text);

//...

fontSize += 10;
textStyles.default.fontSize = fontSize + "px";
text.styles = textStyles;

That said, it'd also be nice to have styles not be write-only.

We should probably also have an error or warning when trying to set style since that has unexpected behavior.

@tleunen
Copy link
Owner

tleunen commented Jan 4, 2017

PixiText.setStyle is deprecated since v3. So I'm not sure it's a good idea that MST provide a similar function.

Having said that, I do believe MST should internally copy the styles so changing them afterwards doesn't affect the rendering. But it also means we have to provide an easy way to change a style.

.style should be allowed, but I like the idea to have an easy helper to only update a specific style tag. So maybe adding .setStyleTag(tag, styles)?

bluepichu added a commit to bluepichu/pixi-multistyle-text that referenced this issue Jan 4, 2017
@bluepichu
Copy link
Collaborator

See c15a1fa for the ES6 approach to this issue. It works... surprisingly well. That said, I feel like dropping ES5 support is probably not the smartest move.

Setting .style directly is kind of a strange case, because it's not entirely clear what it's doing. (Currently, setting that will clear whatever settings you currently have on the global options - align, wordWrap, wordWrapWidth, and breakWords - but will have no other affect on output.)

.setStyleTag() is probably not a bad idea. It's not as pretty as the proxied version of .styles, but it certainly gets the job done. However, in that case we can't allow external access to the underlying style object, as then we could get strange behavior like this:

let mst = new MultiStyleText("<tag>test</tag>", {
    default: { fontFamily: "Arial" },
    tag: { fontSize: "32px" }
};
stage.addChild(mst); // renders as expected
mst.styles.tag.fontSize = "64px"; // still has font size 32px
mst.text = "<tag>what?</tag>"; // now has font size 64px

So if we want to allow read access to .styles, we'll need to come up with something else.

@ArronFerguson
Copy link
Author

This I tried:

fontSize += 10;
textStyles.default.fontSize = fontSize + "px";
text.styles = textStyles;

But unfortunately it doesn't change the text object.

.setStyleTag() is probably not a bad idea

That actually sounds pretty good to me. It would be a nice interface addition IMO.

bluepichu added a commit to bluepichu/pixi-multistyle-text that referenced this issue Jan 5, 2017
@bluepichu
Copy link
Collaborator

PR filed: #35.

@ArronFerguson
Copy link
Author

Awesome, thank you!

@bluepichu bluepichu self-assigned this Jan 14, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants