Simplify viewer's handling of backbuffer surface and color space
WindowContext still supports color spaces, but not other color
types. Any off-screen rendering is the app's responsibility.
This change also adds (working) F16 support to viewer. Note that
the previous 10-bit and FP16 support in WindowContext was broken.
There was no code to push the off-screen canvas to the window.
If you ever made it to the unreachable off-screen code path in
createSurface, it would have simply stopped drawing.
The decision to limit the window's gamut to sRGB is mostly driven
by my desire to add real-time editing of gamut. This design lets
us do that, without tearing down and rebuilding the window for
every change. An application could still supply a different gamut
via setDisplayParams and render directly to the back buffer with
proper color correction.
BUG=skia:
Change-Id: I94df35c7a42faee396009acc83683e40bb3c284d
Reviewed-on: https://skia-review.googlesource.com/8153
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Matt Sarett <msarett@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
diff --git a/tools/viewer/sk_app/DisplayParams.h b/tools/viewer/sk_app/DisplayParams.h
index b9a23f3..0c649c0 100644
--- a/tools/viewer/sk_app/DisplayParams.h
+++ b/tools/viewer/sk_app/DisplayParams.h
@@ -15,13 +15,11 @@
DisplayParams()
: fColorType(kN32_SkColorType)
, fColorSpace(nullptr)
- , fMSAASampleCount(0)
- , fDeepColor(false) {}
+ , fMSAASampleCount(0) {}
SkColorType fColorType;
sk_sp<SkColorSpace> fColorSpace;
int fMSAASampleCount;
- bool fDeepColor;
};
} // namespace sk_app
diff --git a/tools/viewer/sk_app/GLWindowContext.cpp b/tools/viewer/sk_app/GLWindowContext.cpp
index dd51b7e..501f272 100644
--- a/tools/viewer/sk_app/GLWindowContext.cpp
+++ b/tools/viewer/sk_app/GLWindowContext.cpp
@@ -39,11 +39,8 @@
// We may not have real sRGB support (ANGLE, in particular), so check for
// that, and fall back to L32:
- //
- // ... and, if we're using a 10-bit/channel FB0, it doesn't do sRGB conversion on write,
- // so pretend that it's non-sRGB 8888:
- fPixelConfig = fContext->caps()->srgbSupport() && fDisplayParams.fColorSpace &&
- (fColorBits != 30) ? kSRGBA_8888_GrPixelConfig : kRGBA_8888_GrPixelConfig;
+ fPixelConfig = fContext->caps()->srgbSupport() && fDisplayParams.fColorSpace
+ ? kSRGBA_8888_GrPixelConfig : kRGBA_8888_GrPixelConfig;
}
void GLWindowContext::destroyContext() {
@@ -63,8 +60,6 @@
sk_sp<SkSurface> GLWindowContext::getBackbufferSurface() {
if (nullptr == fSurface) {
- fActualColorBits = SkTMax(fColorBits, 24);
-
if (fContext) {
GrBackendRenderTargetDesc desc;
desc.fWidth = this->fWidth;
@@ -77,7 +72,9 @@
GR_GL_CALL(fBackendContext.get(), GetIntegerv(GR_GL_FRAMEBUFFER_BINDING, &buffer));
desc.fRenderTargetHandle = buffer;
- fSurface = this->createRenderSurface(desc, fActualColorBits);
+ fSurface = SkSurface::MakeFromBackendRenderTarget(fContext, desc,
+ fDisplayParams.fColorSpace,
+ &fSurfaceProps);
}
}
diff --git a/tools/viewer/sk_app/GLWindowContext.h b/tools/viewer/sk_app/GLWindowContext.h
index 7a3256eb..a7d1c98 100644
--- a/tools/viewer/sk_app/GLWindowContext.h
+++ b/tools/viewer/sk_app/GLWindowContext.h
@@ -55,11 +55,9 @@
// parameters obtained from the native window
// Note that the platform .cpp file is responsible for
- // initializing fSampleCount, fStencilBits, and fColorBits!
+ // initializing fSampleCount and fStencilBits!
int fSampleCount;
int fStencilBits;
- int fColorBits;
- int fActualColorBits;
};
} // namespace sk_app
diff --git a/tools/viewer/sk_app/VulkanWindowContext.cpp b/tools/viewer/sk_app/VulkanWindowContext.cpp
index 898710f..b4e6676 100644
--- a/tools/viewer/sk_app/VulkanWindowContext.cpp
+++ b/tools/viewer/sk_app/VulkanWindowContext.cpp
@@ -276,7 +276,9 @@
desc.fStencilBits = 0;
desc.fRenderTargetHandle = (GrBackendObject) &info;
- fSurfaces[i] = this->createRenderSurface(desc, 24);
+ fSurfaces[i] = SkSurface::MakeFromBackendRenderTarget(fContext, desc,
+ fDisplayParams.fColorSpace,
+ &fSurfaceProps);
}
// create the command pool for the command buffers
diff --git a/tools/viewer/sk_app/Window.cpp b/tools/viewer/sk_app/Window.cpp
index 4a04363..dec1584 100644
--- a/tools/viewer/sk_app/Window.cpp
+++ b/tools/viewer/sk_app/Window.cpp
@@ -113,8 +113,4 @@
fIsContentInvalidated = false;
}
-sk_sp<SkSurface> Window::getOffscreenSurface(bool forceSRGB) {
- return fWindowContext->createOffscreenSurface(forceSRGB);
-}
-
} // namespace sk_app
diff --git a/tools/viewer/sk_app/Window.h b/tools/viewer/sk_app/Window.h
index 36d5414..ea6f6c1 100644
--- a/tools/viewer/sk_app/Window.h
+++ b/tools/viewer/sk_app/Window.h
@@ -164,9 +164,6 @@
virtual const DisplayParams& getDisplayParams();
void setDisplayParams(const DisplayParams& params);
- // This is just for the sRGB split screen
- sk_sp<SkSurface> getOffscreenSurface(bool forceSRGB);
-
protected:
Window();
diff --git a/tools/viewer/sk_app/WindowContext.cpp b/tools/viewer/sk_app/WindowContext.cpp
deleted file mode 100644
index 8efe9bd..0000000
--- a/tools/viewer/sk_app/WindowContext.cpp
+++ /dev/null
@@ -1,63 +0,0 @@
-
-/*
- * Copyright 2015 Google Inc.
- *
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- */
-
-#include "GrContext.h"
-#include "SkSurface.h"
-#include "WindowContext.h"
-
-#include "gl/GrGLDefines.h"
-
-#include "gl/GrGLUtil.h"
-#include "GrRenderTarget.h"
-#include "GrContext.h"
-
-#include "SkCanvas.h"
-#include "SkImage_Base.h"
-
-namespace sk_app {
-
-sk_sp<SkSurface> WindowContext::createOffscreenSurface(bool forceSRGB) {
- return createSurface(nullptr, 0, true, forceSRGB);
-}
-
-sk_sp<SkSurface> WindowContext::createRenderSurface(const GrBackendRenderTargetDesc& desc,
- int colorBits) {
- return createSurface(&desc, colorBits, false, false);
-}
-
-sk_sp<SkSurface> WindowContext::createSurface(
- const GrBackendRenderTargetDesc* rtDesc, int colorBits, bool offscreen, bool forceSRGB) {
- if (!this->isGpuContext() || colorBits > 24 || offscreen ||
- kRGBA_F16_SkColorType == fDisplayParams.fColorType) {
- // If we're rendering to F16, we need an off-screen surface - the current render
- // target is most likely the wrong format.
- //
- // If we're rendering raster data or using a deep (10-bit or higher) surface, we probably
- // need an off-screen surface. 10-bit, in particular, has strange gamma behavior.
- SkImageInfo info = SkImageInfo::Make(
- fWidth, fHeight,
- fDisplayParams.fColorType,
- kPremul_SkAlphaType,
- forceSRGB ? SkColorSpace::MakeSRGB()
- : fDisplayParams.fColorSpace
- );
- if (this->isGpuContext()) {
- return SkSurface::MakeRenderTarget(fContext, SkBudgeted::kNo, info,
- fDisplayParams.fMSAASampleCount, &fSurfaceProps);
- } else {
- return SkSurface::MakeRaster(info, &fSurfaceProps);
- }
- } else {
- sk_sp<SkColorSpace> colorSpace = GrPixelConfigIsSRGB(rtDesc->fConfig)
- ? SkColorSpace::MakeSRGB() : nullptr;
- return SkSurface::MakeFromBackendRenderTarget(fContext, *rtDesc, colorSpace,
- &fSurfaceProps);
- }
-}
-
-} //namespace sk_app
diff --git a/tools/viewer/sk_app/WindowContext.h b/tools/viewer/sk_app/WindowContext.h
index 438d3c0..75edf2d 100644
--- a/tools/viewer/sk_app/WindowContext.h
+++ b/tools/viewer/sk_app/WindowContext.h
@@ -44,13 +44,9 @@
virtual GrBackendContext getBackendContext() = 0;
GrContext* getGrContext() const { return fContext; }
- sk_sp<SkSurface> createOffscreenSurface(bool sRGB);
-
protected:
virtual bool isGpuContext() { return true; }
- sk_sp<SkSurface> createRenderSurface(const GrBackendRenderTargetDesc&, int colorBits);
-
GrContext* fContext;
int fWidth;
@@ -58,10 +54,6 @@
DisplayParams fDisplayParams;
GrPixelConfig fPixelConfig;
SkSurfaceProps fSurfaceProps;
-
-private:
- sk_sp<SkSurface> createSurface(
- const GrBackendRenderTargetDesc*, int colorBits, bool offscreen, bool forceSRGB);
};
} // namespace sk_app
diff --git a/tools/viewer/sk_app/android/GLWindowContext_android.cpp b/tools/viewer/sk_app/android/GLWindowContext_android.cpp
index 20a7631..7dd813d 100644
--- a/tools/viewer/sk_app/android/GLWindowContext_android.cpp
+++ b/tools/viewer/sk_app/android/GLWindowContext_android.cpp
@@ -123,11 +123,6 @@
glStencilMask(0xffffffff);
glClear(GL_STENCIL_BUFFER_BIT | GL_COLOR_BUFFER_BIT);
- int redBits, greenBits, blueBits;
- eglGetConfigAttrib(fDisplay, surfaceConfig, EGL_RED_SIZE, &redBits);
- eglGetConfigAttrib(fDisplay, surfaceConfig, EGL_GREEN_SIZE, &greenBits);
- eglGetConfigAttrib(fDisplay, surfaceConfig, EGL_BLUE_SIZE, &blueBits);
- fColorBits = redBits + greenBits + blueBits;
eglGetConfigAttrib(fDisplay, surfaceConfig, EGL_STENCIL_SIZE, &fStencilBits);
eglGetConfigAttrib(fDisplay, surfaceConfig, EGL_SAMPLES, &fSampleCount);
}
diff --git a/tools/viewer/sk_app/mac/GLWindowContext_mac.cpp b/tools/viewer/sk_app/mac/GLWindowContext_mac.cpp
index bc86df3..5bdba64 100644
--- a/tools/viewer/sk_app/mac/GLWindowContext_mac.cpp
+++ b/tools/viewer/sk_app/mac/GLWindowContext_mac.cpp
@@ -66,12 +66,6 @@
glStencilMask(0xffffffff);
glClear(GL_STENCIL_BUFFER_BIT | GL_COLOR_BUFFER_BIT);
- int redBits, greenBits, blueBits;
- SDL_GL_GetAttribute(SDL_GL_RED_SIZE, &redBits);
- SDL_GL_GetAttribute(SDL_GL_GREEN_SIZE, &greenBits);
- SDL_GL_GetAttribute(SDL_GL_BLUE_SIZE, &blueBits);
- fColorBits = redBits + greenBits + blueBits;
-
SDL_GL_GetAttribute(SDL_GL_STENCIL_SIZE, &fStencilBits);
SDL_GL_GetAttribute(SDL_GL_MULTISAMPLESAMPLES, &fSampleCount);
diff --git a/tools/viewer/sk_app/mac/RasterWindowContext_mac.cpp b/tools/viewer/sk_app/mac/RasterWindowContext_mac.cpp
index 4749bc7..a88ed13 100644
--- a/tools/viewer/sk_app/mac/RasterWindowContext_mac.cpp
+++ b/tools/viewer/sk_app/mac/RasterWindowContext_mac.cpp
@@ -78,12 +78,6 @@
glStencilMask(0xffffffff);
glClear(GL_STENCIL_BUFFER_BIT | GL_COLOR_BUFFER_BIT);
- int redBits, greenBits, blueBits;
- SDL_GL_GetAttribute(SDL_GL_RED_SIZE, &redBits);
- SDL_GL_GetAttribute(SDL_GL_GREEN_SIZE, &greenBits);
- SDL_GL_GetAttribute(SDL_GL_BLUE_SIZE, &blueBits);
- fColorBits = redBits + greenBits + blueBits;
-
SDL_GL_GetAttribute(SDL_GL_STENCIL_SIZE, &fStencilBits);
SDL_GL_GetAttribute(SDL_GL_MULTISAMPLESAMPLES, &fSampleCount);
@@ -115,21 +109,9 @@
// We made/have an off-screen surface. Get the contents as an SkImage:
sk_sp<SkImage> snapshot = fBackbufferSurface->makeImageSnapshot();
- // With ten-bit output, we need to manually apply the gamma of the output device
- // (unless we're in non-gamma correct mode, in which case our data is already
- // fake-sRGB, like we're expected to put in the 10-bit buffer):
- bool doGamma = (fActualColorBits == 30) &&
- (fDisplayParams.fColorSpace != nullptr ||
- kRGBA_F16_SkColorType == fDisplayParams.fColorType);
- SkPaint gammaPaint;
- gammaPaint.setBlendMode(SkBlendMode::kSrc);
- if (doGamma) {
- gammaPaint.setColorFilter(sk_tool_utils::MakeLinearToSRGBColorFilter());
- }
-
sk_sp<SkSurface> gpuSurface = INHERITED::getBackbufferSurface();
SkCanvas* gpuCanvas = gpuSurface->getCanvas();
- gpuCanvas->drawImage(snapshot, 0, 0, &gammaPaint);
+ gpuCanvas->drawImage(snapshot, 0, 0);
gpuCanvas->flush();
SDL_GL_SwapWindow(fWindow);
diff --git a/tools/viewer/sk_app/unix/GLWindowContext_unix.cpp b/tools/viewer/sk_app/unix/GLWindowContext_unix.cpp
index 6df1a52..ead7037 100644
--- a/tools/viewer/sk_app/unix/GLWindowContext_unix.cpp
+++ b/tools/viewer/sk_app/unix/GLWindowContext_unix.cpp
@@ -61,11 +61,6 @@
glStencilMask(0xffffffff);
glClear(GL_STENCIL_BUFFER_BIT | GL_COLOR_BUFFER_BIT);
- int redBits, greenBits, blueBits;
- glXGetConfig(fDisplay, fVisualInfo, GLX_RED_SIZE, &redBits);
- glXGetConfig(fDisplay, fVisualInfo, GLX_GREEN_SIZE, &greenBits);
- glXGetConfig(fDisplay, fVisualInfo, GLX_BLUE_SIZE, &blueBits);
- fColorBits = redBits + greenBits + blueBits;
glXGetConfig(fDisplay, fVisualInfo, GLX_STENCIL_SIZE, &fStencilBits);
glXGetConfig(fDisplay, fVisualInfo, GLX_SAMPLES_ARB, &fSampleCount);
diff --git a/tools/viewer/sk_app/win/GLWindowContext_win.cpp b/tools/viewer/sk_app/win/GLWindowContext_win.cpp
index bf1ddb9..055abf3 100644
--- a/tools/viewer/sk_app/win/GLWindowContext_win.cpp
+++ b/tools/viewer/sk_app/win/GLWindowContext_win.cpp
@@ -52,7 +52,7 @@
void GLWindowContext_win::onInitializeContext() {
HDC dc = GetDC(fHWND);
- fHGLRC = SkCreateWGLContext(dc, fDisplayParams.fMSAASampleCount, fDisplayParams.fDeepColor,
+ fHGLRC = SkCreateWGLContext(dc, fDisplayParams.fMSAASampleCount, false /* deepColor */,
kGLPreferCompatibilityProfile_SkWGLContextRequest);
if (NULL == fHGLRC) {
return;
@@ -69,8 +69,6 @@
PIXELFORMATDESCRIPTOR pfd;
DescribePixelFormat(dc, pixelFormat, sizeof(pfd), &pfd);
fStencilBits = pfd.cStencilBits;
- // pfd.cColorBits includes alpha, so it will be 32 in 8/8/8/8 and 10/10/10/2
- fColorBits = pfd.cRedBits + pfd.cGreenBits + pfd.cBlueBits;
// Get sample count if the MSAA WGL extension is present
SkWGLExtensions extensions;