This repository was archived by the owner on Dec 5, 2018. It is now read-only.
| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
| Back | FazBrowse Home | New Git URL |
Activity
Pomax commented on Mar 23, 2016
Hopefully, you? Processing.js has not seen active development in quite a while now, so it's entirely likely that there was a plan to add them at some point in the past, but no one got to it before everyone went their separate ways. I mostly curate the repo at this point, so I will certainly mark it as a bug, if someone sees this bug and feels like fixing it then that'll be great, but otherwise the best way to get this fixed is to see if you can fix it yourself and submit a PR to have that fix merged back into the master branch. Otherwise, I might get to this but that could be either this weekend or a year from now, I honestly can't say given the large number of other projects I'm an active developer on rather than curator of.
atgeflu commented on Jul 10, 2016
Looks like I should be able to find a fix for imageMode as well as ellipseMode.
While I'm at it I'll check whether pushStyle and popStyle work for all styles included in the original Processing.
@Pomax In case I find other styles for which it isn't working should I include them with the same fix or keep it separate. Furthermore, while checking the different styles I'll have examples of things that work already but may still be included as ref tests. Should I include them as ref tests for completeness sake or not include examples for working styles at all?
Pomax commented on Jul 10, 2016
if they're related to preserving the style block, tackling them in the same PR is fine, since it's still the same issue. Getting a checklist of bits that all need to be preserved but aren't, before filing a PR, would be a good idea. Ref tests for each of the style components is a good idea, although I'm pretty sure we can make most of those unit tests, since we have access to the currentContext so we can programmatically check what the values are after switching from one context to another, and then back.
Let's aim for unit tests first, with ref test as fallback if unit tests turn out to not be feasible.
atgeflu commented on Jul 12, 2016
@Pomax I've been trying to create some unit tests, but I've been running into some difficulties.
I've tried using the currentContext by calling var curContext = externals.context; from within a pde that I wrote to run some unit tests but I haven't figured out how I could get the values of imageMode or ellipseMode (I believe the current values should be saved in the vars curEllipseModeand imageModeConvert). Getting things like linewidth from the currentContext seems to work, but I don't get the values I'd expect when using pushStyle() and popStyle() and then querying curContext.lineWidth.
Do you know if there is an easy way of getting the values of imageMode or ellipseMode from within a pde sketch?
Pomax commented on Jul 12, 2016
can you describe what you mean when you say "I don't get the values I'd expect"? E.g. if you start with a lineWidth of 1 and then push the style, change the value, then pop the style, does it report a different value than what it actually draws with?
atgeflu commented on Jul 13, 2016
Maybe I'm doing something wrong with the unit test. My unit test for strokeWeightlooks like this:
size(100, 100); var curContext = externals.context; void setup() { strokeWeight(4); // Thicker pushStyle(); strokeWeight(10); // Beastly line(20, 40, 80, 40); _checkEqual(10, curContext.lineWidth); popStyle(); line(20, 70, 80, 70); _checkEqual(4, curContext.lineWidth); }It fails on the second check, telling me that 4 != 10.
If I use essentially the same code on a webpage I get the right results though (both in terms of the line width with which the line is drawn as well as the console output:
size(100,100); var curContext = externals.context; void setup() { strokeWeight(4); // Thicker pushStyle(); strokeWeight(10); // Beastly line(20, 40, 80, 40); console.log("expected 10:" + curContext.lineWidth); popStyle(); line(20, 70, 80, 70); console.log("expected 4:" + curContext.lineWidth); }Pomax commented on Jul 15, 2016
What if you refetch curContext?
void setup() { size(100,100); strokeWeight(4); // verify strokeWeight var curContext = externals.context; var sw = curContext.lineWidth; _checkEqual(sw, 4); pushStyle(); strokeWeight(10); // verify updated strokeWeight curContext = externals.context; var th = curContext.lineWidth; _checkEqual(th,10); popStyle(); // verify restored strokeWeight curContext = externals.context; var csw = curContext.lineWidth; _checkEqual(csw, 4); }atgeflu commented on Jul 15, 2016
That code fails on the third check, telling me that 10 != 4.
Removing all the _checkEqual tests and curContext assignments except for the last one gives me the same error.
I wonder why this doesn't work as a unit test but gives me the right result when running as sketch on a webpage.
Pomax commented on Jul 15, 2016
we might have to do some code tracing, to see where the strokeWeight is getting pulled from - it's possible that it's getting set as initial value but then doesn't get updated in the curContext but does get updated in some other value that is consulted by the stroking code...
atgeflu commented on Jul 15, 2016
I've noticed that I can also use console.log in the unit test pde.
So now using the exact same code:
size(100,100); var curContext = externals.context; void setup() { strokeWeight(4); // Thicker pushStyle(); strokeWeight(10); // Beastly line(20, 40, 80, 40); console.log("expected 10:" + curContext.lineWidth); popStyle(); line(20, 70, 80, 70); console.log("expected 4:" + curContext.lineWidth); }I get different results in the browser and in node test. Chrome, IE and Firefox give correct results only node test doesn't. How does node test emulate a browser? Could the problem be there?
Pomax commented on Jul 15, 2016
it doesn't emulate a browser, it just runs the JS code. So that suggests the strokeWidth is being pulled not from curContext but from the actual "physical" canvas used, accessing its strokeWidth directly - this might require accessible the externals.canvas in that case
atgeflu commented on Jul 15, 2016
Neither externals.canvas nor externals.sketch nor externals.window seem to have lineWidth, strokeWeigtht or strokeWidth so I'm at a loss as to how I can set up a working unit test.
Furthermore the current difficulties aren't even for unit tests for the actual fix. The fix concerns imageMode and ellipseMode both of which I don't think can be accessed through the curContext.
Pomax commented on Jul 16, 2016
Then the first step is to split this up into two different tickets, so that you don't have to solve one problem before you can work on the other.
A good tactic is to look at where strokeWeight is actually pulled from in the code that does the drawing, and then following it back to the source. The drawing is ultimately the canvas, so the value in externals.canvas.getContext("2d").strokeWeight will always be the right one, as that is literally the only value that JavaScript itself consults for Canvas2D graphics. The question is then: does anything bypass the curContext or externals and directly tap into the 2d context value in our code...
But I would recommend making that investigation its own issue, and then for this issue simply saying "we cannot tackle this issue until the investigation in issue xyz has been completed"
atgeflu commented on Jul 16, 2016
Creating a separate ticket for curContext difficulties in the unit tests strike me as a good idea.
So I propose that I create a PR for the push/popStyle fix taking care of imageMode, ellipseMode and shapeMode and add a few ref tests for these.
Then I'll create a new issue for the curContext difficulties and mention that once fixed it'd be a good idea to add unit tests for push/popStyle.
Does this sound like a good plan of action?
Pomax commented on Jul 16, 2016
sounds like a good plan! I also always advocate "file first, then do actual work" because that way you don't forget to file an issue when it's still fresh in your mind.
atgeflu commented on Jul 16, 2016
Good point. So I've opened an issue already (#254).
The PR for the current issue will follow later.
Pomax commented on Aug 1, 2016
apologies for not getting back to this for two weeks; I had some compound hand injuries to deal with.
atgeflu commented on Aug 1, 2016
No worries. Health comes first.
It's a pretty straight forward fix, but it's not running away ;-) so take your time.