there may be concurrency issues. To be honest I wrote the patch just to get things working and I committed it after the fact, so it doesn't have the changes from the patches you were working on.
Also, to be fair my patch still gets the
height issue occasionally as well, so in an attempt to counteract that issue, I added a small delay before retrieving the data URL. However, since I'm waiting a static amount of time, there's still no real guarantee that it will resolve the
width and `height issues.
pop appear to be balanced in all places that call them, which is why the issue was confusing to me in the first place. The ultimate cause of the issue comes down to two different rendering processes being called at once, and the
GlyphContext being replaced in the
Group element itself. The patch I wrote avoids this by not storing the
GlyphContext on the
Group, so it must be passed almost everywhere. In doing this, there's no way for the
GlyphContext on a
Group to be replaced, so the
pop calls seem to remain balanced. But I do agree, that
poping more times than
pushing is an error, and perhaps that should be handled appropriately.
I would imagine that perhaps your original path with the
synchronized keyword addition may also solve the problem, but I haven't tested it thoroughly to be absolutely certain. Since I was already moving forward with the patch I had written and I had a deadline, I didn't have the time to give it it's proper due-diligence yet. Although it still needs to be modified to fix the method Overloading issue. Based on your patch, while renaming the overloaded methods to avoid the error, it does appear that the crash due to a different number of
pops being called is no longer occurring, so that is definitely one way to solve that problem.
When it comes to
height of the rendered output and the
devicePixelRatio of the device, I find this somewhat problematic. The
toDataURL call does. As an example, consider the
SVG output from my example. (In my debug versions I add a dashed line around certain elements to help me with layout).
When submitting through the non-height and width overrides, I get the following results:
And then, when using the width and height overrides, I see the following:
Now this is due in large part, I suspect, to using the default density supplied by Virtual View and having no way to override it. It seems, to me, that the
height should always be multiplied by this density scale, and in order to make the
height aspects work like this, they would need to be multiplied by the desired density. Ultimately it means that the target
density may need to be another field that is passed in, and it would default to the device's. On iOS this is much easier as the ImageContext that is created can automatically be given a scale. Using this information, it may be better, or even necessary, to have the
density live on the
GlyphContext itself (It already stores the width and height).
Please let me know if you have any questions,