CT: color unable to parse color string #13
Comments
|
Caused by lack of a "color Property" and the PhET-iO's inability to handle |
pixelzoom
changed the title
|
Difficult (as usual) to debug in the State wrapper. But it looks like the failure is occurring when ColorIO attempts to deserialize a Color using
Fourier also has no occurrences of |
|
Reproduced locally with |
|
I tried the usual workaround of adding a I need help from someone on the iO team. I have no idea how to narrow this down to a specific Property. |
|
If I make this change to ColorIO, the problem goes away: fromStateObject: stateObject => {
- return stateObject.hex ? new Color( stateObject.hex ) : new Color( stateObject.r, stateObject.g, stateObject.b, stateObject.a );
+ return new Color( stateObject.r, stateObject.g, stateObject.b, stateObject.a );
}Color.toStateObject is: toStateObject() {
return {
r: this.r,
g: this.g,
b: this.b,
a: this.a,
hex: '#' + this.toNumber().toString( 16 )
};
}So when would And has the computation of |
|
|
|
|
|
Discussed with @zepumph via Zoom. The problem is that the color chooser in Studio and color.html only works with CSS strings in hex format. A Color, ColorProfile, and PhetioElementView all need some work.
hex: '#' + this.toNumber().toString( 16 )... and this in return stateObject.hex ? new Color( stateObject.hex ) : new Color( stateObject.r, stateObject.g, stateObject.b, stateObject.a );
let hexColor = this[ key + 'Property' ].value.toNumber().toString( 16 );In colorInput.onchange = () => {
window.phetio.phetioClient.invoke( phetioElement.phetioID, 'setValue', [ { hex: colorInput.value } ] );
};My feeling is that @zepumph is going to take it from here. |
|
Another comment. |
I do not know another way to convert a number to base 16 strings. I think that Number.prototype.toString() is a well vetted and maintained way to achieve this goal. Stack overflow is not an authority, but https://stackoverflow.com/questions/5623838/rgb-to-hex-and-hex-to-rgb serves as a nice data point. I did not see a single implementation in those answers that didn't use Above I committed the bug fix that was actually causing the error reported in this issue. I also factored out |
|
This might be another acceptable solution: Use hex as the serialized representation for Color. In Color.js: toStateObject() {
return {
// Serialize to hex format because other HTML color chooser (used in Studio and color.html) needs hex.
hex: ...
};
}
fromStateObject: stateObject => {
return new Color( stateObject.hex );
} |
…tudio to use Color.js for wrapper-side serialization, phetsims/fourier-making-waves#13 phetsims/scenery-phet#515
From what @samreid and I see, this removes alpha from the implementation. We felt better about keeping rgba as the serialization, and then adding logic to studio to support the conversion, as @pixelzoom and I discussed doing this morning. I think I prefer having an explicit rgba as the single serialization. This implementation involved using ColorIO serialization methods in the wrapper, but @samreid and I felt like that was acceptable and avoided duplicated code. @pixelzoom will you please review. |
|
👍🏻 Looks good, closing. |
The text was updated successfully, but these errors were encountered: