-
-
Notifications
You must be signed in to change notification settings - Fork 75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement API for converting Picture to BufferedImage #144
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there are few issues that need to be looked at before this is ready to go.
w.bufferedImage(frame, picture) | ||
} | ||
|
||
def bufferedImage[Fmt <: Format] = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the Fmt
type is needed. This type specifies a file format (like PNG or JPEG), which is not relevant for saving to an in-memory bitmap.
If the Fmt
types goes the helper classes are no longer necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just added the format becuase the renderBufferedImage()
requires a makeImage()
function which defined for each Java2d[Format]Writer
object
Should I just create a separate object for the BufferedImageConverter
with its own makeImage()
function and make this the only implicit BufferedImageConverter
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this compile on the Javascript platform? E.g. does coreJS / fastOptJS
succeed?
I suspect the BufferedImage
type doesn't exist on JS, and hence it won't compile. This mean either:
- this type needs to become generic in the bitmap type or
- this type needs to migrate to live only on the JVM platform (and possibly only on the Java2D backend.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are probably right. I was just following what Base64
does. I will move it to the Java2D specific directory.
👍 Ok, I think this is good enough to merge. Thanks! There are some points to consider for improvements which I collected in #145 |
closes #87