Skip to content
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

Raster #56

Merged
merged 5 commits into from Feb 1, 2021
Merged

Raster #56

merged 5 commits into from Feb 1, 2021

Conversation

Rsedaikin
Copy link
Contributor

No description provided.

@@ -15,6 +15,7 @@ import kotlin.math.cos
import kotlin.math.sin

fun main(args: Array<String>) {
// System.setProperty("skiko.renderApi", "RASTER")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or maybe move to Gradle script in commented out form.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is better to remove this because this is only for software rendering testing.

skijaState.apply {
canvas!!.clear(bleachConstant)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this code actually belongs to draw? Can we avoid reinit on every frame?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To create a skia context, the OpenGL context must be current, so basically the first time we can create a skia context is inside the draw() method and we cannot initialize the skia context separately due to the macos CAOpenGLLayer update pipeline specific.
initContext performs only once, and all subsequent calls to the method simply return true.

return when (renderApi) {
GraphicsApi.RASTER -> RasterContextHandler(layer)
GraphicsApi.OPENGL -> OpenGLContextHandler(layer)
GraphicsApi.METAL -> MetalContextHandler(layer)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shalln't we throw an exception here?

Copy link
Contributor Author

@Rsedaikin Rsedaikin Jan 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to remove the METAL because it is not ready to use yet.

Copy link
Collaborator

@igordmn igordmn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good!


val bytes = storage.readPixels(storage.getImageInfo(), (w * 4).toLong(), 0, 0)
if (bytes != null) {
val buffer = DataBufferByte(bytes, bytes.size)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot we make custom buffer on top of storage to avoid intermediate bytes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I talking about that with Nikita and looks like we have for now only one way to get image data - via Bitmap.readPixels. I think there are several ways to get a common byte array for skia and awt rendering, but actually reading pixels is negligible compared to drawing (skia + awt). The best we could get is plus 0.5 frames per second. So i decided to do it later.

@@ -1,5 +1,5 @@
package org.jetbrains.skiko

enum class GraphicsApi {
UNKNOWN, OPENGL, VULKAN, METAL
UNKNOWN, RASTER, OPENGL, D3D, VULKAN, METAL
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe names like SOFTWARE and DIRECT3D would be more descriptive.

@Rsedaikin Rsedaikin merged commit 6ad0a4e into master Feb 1, 2021
@Rsedaikin Rsedaikin deleted the raster branch February 1, 2021 07:33
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.

None yet

3 participants