Skip to content

Fix push/pop for text#1321

Merged
lmccart merged 3 commits intoprocessing:masterfrom
islemaster:fix-push-pop-for-text
Mar 25, 2016
Merged

Fix push/pop for text#1321
lmccart merged 3 commits intoprocessing:masterfrom
islemaster:fix-push-pop-for-text

Conversation

@islemaster
Copy link
Copy Markdown
Contributor

Fixes #1320

Make p5.prototype.push() track the _strokeSet and _fillSet properties which can impact text rendering. Adds a simple test to show that these properties are now saved and restored to their original state by push/pop.

These two renderer properties were being skipped by the fill method, even though they affect text rendering.

Fixes processing#1320
@islemaster
Copy link
Copy Markdown
Contributor Author

Note: I've realized I forgot to add my new test file to test.html, so they don't actually run - and when I did add them they don't work yet. Fixing now...

Adds the new unit test file to the test index files so that they run.

Instead of checking that particular properties are preserved by push/pop, run a deepEqual check against the render state before and after each of the operations that push is documented to preserve.  This should have a better chance of catching future subtle breaks to push/pop behavior.  Verified that the new tests actually catch the problem with _strokeSet and _fillSet that I was originally trying to fix.
@islemaster
Copy link
Copy Markdown
Contributor Author

That's better! The tests actually run now 😨 and I replaced the very invasive/targeted tests I had with a more thorough approach that does a deep comparison of the render state before and after using push/pop to undo a particular operation. I added a test for every operation that's supposedly supported, according to the documentation on push() (except for textMode which apparently isn't a real thing?).

@lmccart lmccart merged commit 355eb25 into processing:master Mar 25, 2016
@lmccart
Copy link
Copy Markdown
Member

lmccart commented Mar 25, 2016

right on, thank you!

islemaster added a commit to islemaster/p5.play that referenced this pull request Apr 29, 2016
Previously at 0.4.21. No other changes to code obviously required; tests still pass.

0.4.24 includes [this bugfix](processing/p5.js#1321) which should resolve issue quinton-ashley#54 with Sprite.debug drawing.

See 0.4.24 release notes here: https://github.com/processing/p5.js/releases/tag/0.4.24
@islemaster islemaster deleted the fix-push-pop-for-text branch May 4, 2016 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants