From ccc06b6f5150f6728df6b3dbb8eaa0f9572cf37d Mon Sep 17 00:00:00 2001 From: Peiyong Lin Date: Tue, 25 Jun 2019 17:31:09 -0700 Subject: [PATCH] Attach color space information when attach and queue buffer. Currently when calling attachAndQueueBuffer, the color space information is lost. This results in color shift if the color space doesn't match the color space of the surface. BUG: b/135002842, b/131928312 Test: boot. Manually verified on P19 Change-Id: I95ec73c24942f79197d25ee85f139b2aaf805677 --- core/java/android/view/Surface.java | 37 ++++++++++++++----- core/java/android/view/SurfaceControl.java | 3 +- core/jni/android_view_Surface.cpp | 28 ++++++++++++-- .../server/wm/ScreenRotationAnimation.java | 3 +- .../server/wm/TaskScreenshotAnimatable.java | 2 +- .../server/wm/TaskSnapshotSurface.java | 15 ++++---- 6 files changed, 65 insertions(+), 23 deletions(-) diff --git a/core/java/android/view/Surface.java b/core/java/android/view/Surface.java index cb64ab1fd921d..17f07b5a2ad40 100644 --- a/core/java/android/view/Surface.java +++ b/core/java/android/view/Surface.java @@ -21,6 +21,7 @@ import android.annotation.NonNull; import android.annotation.UnsupportedAppUsage; import android.content.res.CompatibilityInfo.Translator; import android.graphics.Canvas; +import android.graphics.ColorSpace; import android.graphics.GraphicBuffer; import android.graphics.Matrix; import android.graphics.RecordingCanvas; @@ -80,7 +81,8 @@ public class Surface implements Parcelable { private static native long nativeGetNextFrameNumber(long nativeObject); private static native int nativeSetScalingMode(long nativeObject, int scalingMode); private static native int nativeForceScopedDisconnect(long nativeObject); - private static native int nativeAttachAndQueueBuffer(long nativeObject, GraphicBuffer buffer); + private static native int nativeAttachAndQueueBufferWithColorSpace(long nativeObject, + GraphicBuffer buffer, int colorSpaceId); private static native int nativeSetSharedBufferModeEnabled(long nativeObject, boolean enabled); private static native int nativeSetAutoRefreshEnabled(long nativeObject, boolean enabled); @@ -699,18 +701,35 @@ public class Surface implements Parcelable { } /** + * Transfer ownership of buffer with a color space and present it on the Surface. + * The supported color spaces are SRGB and Display P3, other color spaces will be + * treated as SRGB. + * @hide + */ + public void attachAndQueueBufferWithColorSpace(GraphicBuffer buffer, ColorSpace colorSpace) { + synchronized (mLock) { + checkNotReleasedLocked(); + if (colorSpace == null) { + colorSpace = ColorSpace.get(ColorSpace.Named.SRGB); + } + int err = nativeAttachAndQueueBufferWithColorSpace(mNativeObject, buffer, + colorSpace.getId()); + if (err != 0) { + throw new RuntimeException( + "Failed to attach and queue buffer to Surface (bad object?), " + + "native error: " + err); + } + } + } + + /** + * Deprecated, use attachAndQueueBufferWithColorSpace instead. * Transfer ownership of buffer and present it on the Surface. + * The color space of the buffer is treated as SRGB. * @hide */ public void attachAndQueueBuffer(GraphicBuffer buffer) { - synchronized (mLock) { - checkNotReleasedLocked(); - int err = nativeAttachAndQueueBuffer(mNativeObject, buffer); - if (err != 0) { - throw new RuntimeException( - "Failed to attach and queue buffer to Surface (bad object?)"); - } - } + attachAndQueueBufferWithColorSpace(buffer, ColorSpace.get(ColorSpace.Named.SRGB)); } /** diff --git a/core/java/android/view/SurfaceControl.java b/core/java/android/view/SurfaceControl.java index 63e14853b51d2..3c045f4fdb88e 100644 --- a/core/java/android/view/SurfaceControl.java +++ b/core/java/android/view/SurfaceControl.java @@ -1866,7 +1866,8 @@ public final class SurfaceControl implements Parcelable { final ScreenshotGraphicBuffer buffer = screenshotToBuffer(display, sourceCrop, width, height, useIdentityTransform, rotation); try { - consumer.attachAndQueueBuffer(buffer.getGraphicBuffer()); + consumer.attachAndQueueBufferWithColorSpace(buffer.getGraphicBuffer(), + buffer.getColorSpace()); } catch (RuntimeException e) { Log.w(TAG, "Failed to take screenshot - " + e.getMessage()); } diff --git a/core/jni/android_view_Surface.cpp b/core/jni/android_view_Surface.cpp index ccadc7d7c22a8..ff14a2acc4d77 100644 --- a/core/jni/android_view_Surface.cpp +++ b/core/jni/android_view_Surface.cpp @@ -75,6 +75,24 @@ static struct { jfieldID bottom; } gRectClassInfo; +class JNamedColorSpace { +public: + // ColorSpace.Named.SRGB.ordinal() = 0; + static constexpr jint SRGB = 0; + + // ColorSpace.Named.DISPLAY_P3.ordinal() = 7; + static constexpr jint DISPLAY_P3 = 7; +}; + +constexpr ui::Dataspace fromNamedColorSpaceValueToDataspace(const jint colorSpace) { + switch (colorSpace) { + case JNamedColorSpace::DISPLAY_P3: + return ui::Dataspace::DISPLAY_P3; + default: + return ui::Dataspace::V0_SRGB; + } +} + // ---------------------------------------------------------------------------- // this is just a pointer we use to pass to inc/decStrong @@ -425,11 +443,12 @@ static jint nativeForceScopedDisconnect(JNIEnv *env, jclass clazz, jlong nativeO return surface->disconnect(-1, IGraphicBufferProducer::DisconnectMode::AllLocal); } -static jint nativeAttachAndQueueBuffer(JNIEnv *env, jclass clazz, jlong nativeObject, - jobject graphicBuffer) { +static jint nativeAttachAndQueueBufferWithColorSpace(JNIEnv *env, jclass clazz, jlong nativeObject, + jobject graphicBuffer, jint colorSpaceId) { Surface* surface = reinterpret_cast(nativeObject); sp bp = graphicBufferForJavaObject(env, graphicBuffer); - int err = Surface::attachAndQueueBuffer(surface, bp); + int err = Surface::attachAndQueueBufferWithDataspace(surface, bp, + fromNamedColorSpaceValueToDataspace(colorSpaceId)); return err; } @@ -531,7 +550,8 @@ static const JNINativeMethod gSurfaceMethods[] = { {"nativeGetNextFrameNumber", "(J)J", (void*)nativeGetNextFrameNumber }, {"nativeSetScalingMode", "(JI)I", (void*)nativeSetScalingMode }, {"nativeForceScopedDisconnect", "(J)I", (void*)nativeForceScopedDisconnect}, - {"nativeAttachAndQueueBuffer", "(JLandroid/graphics/GraphicBuffer;)I", (void*)nativeAttachAndQueueBuffer}, + {"nativeAttachAndQueueBufferWithColorSpace", "(JLandroid/graphics/GraphicBuffer;I)I", + (void*)nativeAttachAndQueueBufferWithColorSpace}, {"nativeSetSharedBufferModeEnabled", "(JZ)I", (void*)nativeSetSharedBufferModeEnabled}, {"nativeSetAutoRefreshEnabled", "(JZ)I", (void*)nativeSetAutoRefreshEnabled}, diff --git a/services/core/java/com/android/server/wm/ScreenRotationAnimation.java b/services/core/java/com/android/server/wm/ScreenRotationAnimation.java index b90d60227be43..6fd802d10e986 100644 --- a/services/core/java/com/android/server/wm/ScreenRotationAnimation.java +++ b/services/core/java/com/android/server/wm/ScreenRotationAnimation.java @@ -279,7 +279,8 @@ class ScreenRotationAnimation { mService.mDisplayManagerInternal.screenshot(displayId); if (gb != null) { try { - surface.attachAndQueueBuffer(gb.getGraphicBuffer()); + surface.attachAndQueueBufferWithColorSpace(gb.getGraphicBuffer(), + gb.getColorSpace()); } catch (RuntimeException e) { Slog.w(TAG, "Failed to attach screenshot - " + e.getMessage()); } diff --git a/services/core/java/com/android/server/wm/TaskScreenshotAnimatable.java b/services/core/java/com/android/server/wm/TaskScreenshotAnimatable.java index ee4e462cb85e1..bb1570e1ecb1b 100644 --- a/services/core/java/com/android/server/wm/TaskScreenshotAnimatable.java +++ b/services/core/java/com/android/server/wm/TaskScreenshotAnimatable.java @@ -69,7 +69,7 @@ class TaskScreenshotAnimatable implements SurfaceAnimator.Animatable { if (buffer != null) { final Surface surface = new Surface(); surface.copyFrom(mSurfaceControl); - surface.attachAndQueueBuffer(buffer); + surface.attachAndQueueBufferWithColorSpace(buffer, screenshotBuffer.getColorSpace()); surface.release(); } getPendingTransaction().show(mSurfaceControl); diff --git a/services/core/java/com/android/server/wm/TaskSnapshotSurface.java b/services/core/java/com/android/server/wm/TaskSnapshotSurface.java index c3762d6352b8f..1905476f4729e 100644 --- a/services/core/java/com/android/server/wm/TaskSnapshotSurface.java +++ b/services/core/java/com/android/server/wm/TaskSnapshotSurface.java @@ -284,7 +284,6 @@ class TaskSnapshotSurface implements StartingSurface { } private void drawSnapshot() { - final GraphicBuffer buffer = mSnapshot.getSnapshot(); mSurface.copyFrom(mSurfaceControl); if (DEBUG_STARTING_WINDOW) Slog.v(TAG, "Drawing snapshot surface sizeMismatch=" @@ -293,9 +292,9 @@ class TaskSnapshotSurface implements StartingSurface { // The dimensions of the buffer and the window don't match, so attaching the buffer // will fail. Better create a child window with the exact dimensions and fill the parent // window with the background color! - drawSizeMismatchSnapshot(buffer); + drawSizeMismatchSnapshot(); } else { - drawSizeMatchSnapshot(buffer); + drawSizeMatchSnapshot(); } synchronized (mService.mGlobalLock) { mShownTime = SystemClock.uptimeMillis(); @@ -307,15 +306,17 @@ class TaskSnapshotSurface implements StartingSurface { mSnapshot = null; } - private void drawSizeMatchSnapshot(GraphicBuffer buffer) { - mSurface.attachAndQueueBuffer(buffer); + private void drawSizeMatchSnapshot() { + mSurface.attachAndQueueBufferWithColorSpace(mSnapshot.getSnapshot(), + mSnapshot.getColorSpace()); mSurface.release(); } - private void drawSizeMismatchSnapshot(GraphicBuffer buffer) { + private void drawSizeMismatchSnapshot() { if (!mSurface.isValid()) { throw new IllegalStateException("mSurface does not hold a valid surface."); } + final GraphicBuffer buffer = mSnapshot.getSnapshot(); final SurfaceSession session = new SurfaceSession(); // We consider nearly matched dimensions as there can be rounding errors and the user won't // notice very minute differences from scaling one dimension more than the other @@ -355,7 +356,7 @@ class TaskSnapshotSurface implements StartingSurface { } finally { SurfaceControl.closeTransaction(); } - surface.attachAndQueueBuffer(buffer); + surface.attachAndQueueBufferWithColorSpace(buffer, mSnapshot.getColorSpace()); surface.release(); if (aspectRatioMismatch) {