On Fri, 4 Apr 2025 21:46:02 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>>> We are not ready for JSON logs, I admit, but this was the rationale behind
>>> the design of this class:
>>>
>>> * a low-level method that returns byte[]
>>> * a base-64 encoding method that allows for custom prefix/suffix to be able
>>> to do a data url or a json
>>> * a convenience method to use in tests
>>
>> Are you saying that in a JSON image, you wouldn't have the
>> `""data:image/png;base64,"` prefix?
>>
>>> Also, considering what you said about gradle and stderr, perhaps
>>> `writeScreenshot()` should always emit to stderr.
>>
>> That seems reasonable. We can always add an overload that takes a
>> PrintStream if needed later.
>
>> in a JSON image, you wouldn't have the `""data:image/png;base64,"` prefix?
>
> correct, the prefix would be something else, like
> `takeScreenshotBase64("{screenshot:","}");`
Are you sure? How would it know it was PNG (and not JPG or BMP)?
In any case, if there is a possibility that the caller might not want the
base64-encoded image to always be prefixed with "data:image/png;base64," then
passing it in is OK. Honestly, though, this is a test utility, so I don't image
it will ever get used.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1746#discussion_r2029473255
