Remove alpha/depth/stencil/antialias from WGC3D::Attributes.

third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp

Index: third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp
diff --git a/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp b/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp
index ab504062f627b280d82d5475ec062c5aaa88e8ab..1acecd87138735214358986bac8450012da7b20d 100644
--- a/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp
+++ b/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp
@@ -90,7 +90,7 @@ static bool shouldFailDrawingBufferCreationForTesting = false;
 
 } // namespace
 
-PassRefPtr<DrawingBuffer> DrawingBuffer::create(PassOwnPtr<WebGraphicsContext3DProvider> contextProvider, const IntSize& size, bool premultipliedAlpha, PreserveDrawingBuffer preserve, WebGraphicsContext3D::Attributes requestedAttributes)
+PassRefPtr<DrawingBuffer> DrawingBuffer::create(PassOwnPtr<WebGraphicsContext3DProvider> contextProvider, const IntSize& size, bool premultipliedAlpha, bool wantAlphaChannel, bool wantDepthBuffer, bool wantStencilBuffer, bool wantAntialiasing, PreserveDrawingBuffer preserve)
 {
     ASSERT(contextProvider);
 
@@ -106,8 +106,10 @@ PassRefPtr<DrawingBuffer> DrawingBuffer::create(PassOwnPtr<WebGraphicsContext3DP
     }
     ASSERT(extensionsUtil->supportsExtension("GL_OES_packed_depth_stencil"));
     extensionsUtil->ensureExtensionEnabled("GL_OES_packed_depth_stencil");
-    bool multisampleSupported = (extensionsUtil->supportsExtension("GL_CHROMIUM_framebuffer_multisample")
-        || extensionsUtil->supportsExtension("GL_EXT_multisampled_render_to_texture"))
+    bool multisampleSupported =
+        wantAntialiasing

[danakj, 2016/03/31 20:27:57]

This is where we disable MSAA if WebGL app said no.

+        && (extensionsUtil->supportsExtension("GL_CHROMIUM_framebuffer_multisample")
+            || extensionsUtil->supportsExtension("GL_EXT_multisampled_render_to_texture"))
         && extensionsUtil->supportsExtension("GL_OES_rgb8_rgba8");
     if (multisampleSupported) {
         extensionsUtil->ensureExtensionEnabled("GL_OES_rgb8_rgba8");
@@ -120,8 +122,8 @@ PassRefPtr<DrawingBuffer> DrawingBuffer::create(PassOwnPtr<WebGraphicsContext3DP
     if (discardFramebufferSupported)
         extensionsUtil->ensureExtensionEnabled("GL_EXT_discard_framebuffer");
 
-    RefPtr<DrawingBuffer> drawingBuffer = adoptRef(new DrawingBuffer(std::move(contextProvider), extensionsUtil.release(), multisampleSupported, discardFramebufferSupported, premultipliedAlpha, preserve, requestedAttributes));
-    if (!drawingBuffer->initialize(size)) {
+    RefPtr<DrawingBuffer> drawingBuffer = adoptRef(new DrawingBuffer(std::move(contextProvider), extensionsUtil.release(), discardFramebufferSupported, premultipliedAlpha, preserve));
+    if (!drawingBuffer->initialize(size, wantAlphaChannel, wantDepthBuffer, wantStencilBuffer, multisampleSupported)) {
         drawingBuffer->beginDestruction();
         return PassRefPtr<DrawingBuffer>();
     }
@@ -133,7 +135,12 @@ void DrawingBuffer::forceNextDrawingBufferCreationToFail()
     shouldFailDrawingBufferCreationForTesting = true;
 }
 
-DrawingBuffer::DrawingBuffer(PassOwnPtr<WebGraphicsContext3DProvider> contextProvider, PassOwnPtr<Extensions3DUtil> extensionsUtil, bool multisampleExtensionSupported, bool discardFramebufferSupported, bool premultipliedAlpha, PreserveDrawingBuffer preserve, WebGraphicsContext3D::Attributes requestedAttributes)
+DrawingBuffer::DrawingBuffer(
+    PassOwnPtr<WebGraphicsContext3DProvider> contextProvider,
+    PassOwnPtr<Extensions3DUtil> extensionsUtil,
+    bool discardFramebufferSupported,
+    bool premultipliedAlpha,
+    PreserveDrawingBuffer preserve)
     : m_preserveDrawingBuffer(preserve)
     , m_scissorEnabled(false)
     , m_texture2DBinding(0)
@@ -145,10 +152,12 @@ DrawingBuffer::DrawingBuffer(PassOwnPtr<WebGraphicsContext3DProvider> contextPro
     , m_gl(m_contextProvider->contextGL())
     , m_extensionsUtil(std::move(extensionsUtil))
     , m_size(-1, -1)
-    , m_requestedAttributes(requestedAttributes)
-    , m_multisampleExtensionSupported(multisampleExtensionSupported)
     , m_discardFramebufferSupported(discardFramebufferSupported)
     , m_premultipliedAlpha(premultipliedAlpha)
+    , m_hasAlphaChannel(false)
+    , m_hasDepthBuffer(false)
+    , m_hasStencilBuffer(false)
+    , m_hasImplicitStencilBuffer(false)
     , m_fbo(0)
     , m_depthStencilBuffer(0)
     , m_multisampleFBO(0)
@@ -268,7 +277,7 @@ bool DrawingBuffer::prepareMailbox(WebExternalTextureMailbox* outMailbox, WebExt
         bitmap->setSize(size());
 
         unsigned char* pixels = bitmap->pixels();
-        bool needPremultiply = m_actualAttributes.alpha && !m_premultipliedAlpha;
+        bool needPremultiply = m_hasAlphaChannel && !m_premultipliedAlpha;
         WebGLImageConversion::AlphaOp op = needPremultiply ? WebGLImageConversion::AlphaDoPremultiply : WebGLImageConversion::AlphaDoNothing;
         if (pixels)
             readBackFramebuffer(pixels, size().width(), size().height(), ReadbackSkia, op);
@@ -283,7 +292,7 @@ bool DrawingBuffer::prepareMailbox(WebExternalTextureMailbox* outMailbox, WebExt
 
     // No buffer available to recycle, create a new one.
     if (!frontColorBufferMailbox) {
-        frontColorBufferMailbox = createNewMailbox(createTextureAndAllocateMemory(m_size));
+        frontColorBufferMailbox = createNewMailbox(createTextureAndAllocateMemory(m_size, m_hasAlphaChannel));

[danakj, 2016/03/31 20:27:57]

This is a possible behaviour change tho I'm not sure.

The old behaviour would use the m_requestedAttributes to inform if we want RGB
or RGBA.

The new behaviour effectively uses the m_actualAttributes to inform if we want
RGB or RGBA.

IOW we will continue to get RGB or RGBA based on what happened in initialize().
But would that change?

[piman, 2016/03/31 20:54:59]

Can we DCHECK they're the same? In particular, if we asked for RGB, if we were
to get alpha bits, we definitely want to keep telling the compositor that it's
opaque, so that we get culling, turn off blending, etc.

I think today they're the same, but see TODO on l.975 where we want to emulate
RGB on top of RGBA on some platforms that don't support RGB native buffers.

     }
 
     if (m_preserveDrawingBuffer == Discard) {
@@ -347,7 +356,7 @@ void DrawingBuffer::mailboxReleased(const WebExternalTextureMailbox& mailbox, bo
     ASSERT_NOT_REACHED();
 }
 
-DrawingBuffer::TextureParameters DrawingBuffer::chromiumImageTextureParameters()
+DrawingBuffer::TextureParameters DrawingBuffer::chromiumImageTextureParameters(bool wantAlphaChannel)
 {
 #if OS(MACOSX)
     // A CHROMIUM_image backed texture requires a specialized set of parameters
@@ -359,15 +368,15 @@ DrawingBuffer::TextureParameters DrawingBuffer::chromiumImageTextureParameters()
     parameters.colorFormat = GL_RGBA;
     return parameters;
 #else
-    return defaultTextureParameters();
+    return defaultTextureParameters(wantAlphaChannel);
 #endif
 }
 
-DrawingBuffer::TextureParameters DrawingBuffer::defaultTextureParameters()
+DrawingBuffer::TextureParameters DrawingBuffer::defaultTextureParameters(bool wantAlphaChannel)
 {
     TextureParameters parameters;
     parameters.target = GL_TEXTURE_2D;
-    if (m_requestedAttributes.alpha) {
+    if (wantAlphaChannel) {
         parameters.internalColorFormat = GL_RGBA;
         parameters.colorFormat = GL_RGBA;
         parameters.internalRenderbufferFormat = GL_RGBA8_OES;
@@ -449,7 +458,7 @@ void DrawingBuffer::deleteMailbox(const WebExternalTextureMailbox& mailbox)
     ASSERT_NOT_REACHED();
 }
 
-bool DrawingBuffer::initialize(const IntSize& size)
+bool DrawingBuffer::initialize(const IntSize& size, bool wantAlphaChannel, bool wantDepthBuffer, bool wantStencilBuffer, bool multisampleExtensionSupported)
 {
     if (m_gl->GetGraphicsResetStatusKHR() != GL_NO_ERROR) {
         // Need to try to restore the context again later.
@@ -458,9 +467,12 @@ bool DrawingBuffer::initialize(const IntSize& size)
 
     m_gl->GetIntegerv(GL_MAX_TEXTURE_SIZE, &m_maxTextureSize);
 
+    // We update our expectations as we initialize to what we actually get from
+    // the GL context.
+
     int maxSampleCount = 0;
     m_antiAliasingMode = None;
-    if (m_requestedAttributes.antialias && m_multisampleExtensionSupported) {
+    if (multisampleExtensionSupported) {
         m_gl->GetIntegerv(GL_MAX_SAMPLES_ANGLE, &maxSampleCount);
         m_antiAliasingMode = MSAAExplicitResolve;
         if (m_extensionsUtil->supportsExtension("GL_EXT_multisampled_render_to_texture")) {
@@ -475,27 +487,29 @@ bool DrawingBuffer::initialize(const IntSize& size)
 
     m_gl->BindFramebuffer(GL_FRAMEBUFFER, m_fbo);
     createSecondaryBuffers();
-    // We first try to initialize everything with the requested attributes.
-    if (!reset(size))
+    // We first try to initialize everything as requested.
+    if (!reset(size, wantAlphaChannel, wantDepthBuffer || wantStencilBuffer))
         return false;
-    // If that succeeds, we then see what we actually got and update our actual attributes to reflect that.
-    m_actualAttributes = m_requestedAttributes;
-    if (m_requestedAttributes.alpha) {
+    // Then update our state with what we got from the GL context.
+    if (m_depthStencilBuffer && !wantStencilBuffer) {
+        DCHECK(wantDepthBuffer);
+        m_hasImplicitStencilBuffer = true;

[danakj, 2016/03/31 20:27:57]

This replaces the hasImplicitStencilBuffer() function, now it just returns this
bool.

It used to check that wantDepthBuffer was true, but we don't actually need to,
we should not get m_depthStencilBuffer if it wasn't and wantStencilBuffer
wasn't. So DCHECK instead.

+    }
+    if (wantAlphaChannel) {
         GLint alphaBits = 0;
         m_gl->GetIntegerv(GL_ALPHA_BITS, &alphaBits);

[danakj, 2016/03/31 20:41:16]

Oh, these seem to be wrong maybe now. They read about the default buffer not
about the texture we created in reset()? Any suggestions?

[danakj, 2016/03/31 20:46:43]

For alpha.. we just use RGB or RGBA and we use that for texImage2D and stuff,
should we just assume we have alpha on the texture if we want it?

For depth/stencil, should we consider them true if m_depthStencilBuffer is
non-zero?

[piman, 2016/03/31 21:07:49]

Hit send before seeing these questions. The values should be for the currently
bound FBO, not the default framebuffer - so they give you the right things. But
yes these queries seem not particularly useful (and costly - each one is a round
trip) - we should know what we asked for. These made more sense when we used the
default framebuffer, and we didn't know what we were getting.

[danakj, 2016/03/31 21:57:24]

OK IIUC from all this and from IRC, then what I am going to do is the following:

0. Remove all these 3 queries.

1. Store m_wantAlphaChannel on DrawingBuffer, which says if we want A or not.

This will not reflect the result of a query of GL, but rather just what texture
format we're allocating. It can't change after construction, so I've made it
const.

The assumption then is that none of the code cares if what we got from GL
doesn't match what we requested (or that this isn't possible).

2. Store m_hasDepthBuffer/m_hasStencilBuffer/m_hasImplicitStencilBuffer on
DrawingBuffer.

The first 2 are set to true IFF you requested them, and we allocated them.
The latter is set to true IFF we didn't request it, and we allocated it.

The assumption then is that if we Gen/Bind/FramebufferRenderbuffer for the
STENCIL_DEPTH buffer, it would have a non-0 number of bits if we'd queried for
them.

3. I stopped plumbing wantAlphaChannel all over now, since it's a member.


Please check my assumptions.

-        m_actualAttributes.alpha = alphaBits > 0;
+        m_hasAlphaChannel = alphaBits > 0;
     }
-    if (m_requestedAttributes.depth) {
+    if (wantDepthBuffer) {
         GLint depthBits = 0;
         m_gl->GetIntegerv(GL_DEPTH_BITS, &depthBits);
-        m_actualAttributes.depth = depthBits > 0;
+        m_hasDepthBuffer = depthBits > 0;
     }
-    if (m_requestedAttributes.stencil) {
+    if (wantStencilBuffer) {
         GLint stencilBits = 0;
         m_gl->GetIntegerv(GL_STENCIL_BITS, &stencilBits);
-        m_actualAttributes.stencil = stencilBits > 0;
+        m_hasStencilBuffer = stencilBits > 0;
     }
-    m_actualAttributes.antialias = multisample();
 
     if (m_gl->GetGraphicsResetStatusKHR() != GL_NO_ERROR) {
         // It's possible that the drawing buffer allocation provokes a context loss, so check again just in case. http://crbug.com/512302
@@ -545,9 +559,9 @@ bool DrawingBuffer::copyToPlatformTexture(WebGraphicsContext3D* context, gpu::gl
 
     GLboolean unpackPremultiplyAlphaNeeded = GL_FALSE;
     GLboolean unpackUnpremultiplyAlphaNeeded = GL_FALSE;
-    if (m_actualAttributes.alpha && m_premultipliedAlpha && !premultiplyAlpha)
+    if (m_hasAlphaChannel && m_premultipliedAlpha && !premultiplyAlpha)
         unpackUnpremultiplyAlphaNeeded = GL_TRUE;
-    else if (m_actualAttributes.alpha && !m_premultipliedAlpha && premultiplyAlpha)
+    else if (m_hasAlphaChannel && !m_premultipliedAlpha && premultiplyAlpha)
         unpackPremultiplyAlphaNeeded = GL_TRUE;
 
     gl->CopyTextureCHROMIUM(sourceTexture, texture, internalFormat, destType, flipY, unpackPremultiplyAlphaNeeded, unpackUnpremultiplyAlphaNeeded);
@@ -574,8 +588,8 @@ WebLayer* DrawingBuffer::platformLayer()
     if (!m_layer) {
         m_layer = adoptPtr(Platform::current()->compositorSupport()->createExternalTextureLayer(this));
 
-        m_layer->setOpaque(!m_actualAttributes.alpha);
-        m_layer->setBlendBackgroundColor(m_actualAttributes.alpha);
+        m_layer->setOpaque(!m_hasAlphaChannel);
+        m_layer->setBlendBackgroundColor(m_hasAlphaChannel);
         m_layer->setPremultipliedAlpha(m_premultipliedAlpha);
         m_layer->setNearestNeighbor(m_filterQuality == kNone_SkFilterQuality);
         GraphicsLayer::registerContentsLayer(m_layer->layer());
@@ -654,10 +668,10 @@ void DrawingBuffer::createSecondaryBuffers()
     }
 }
 
-bool DrawingBuffer::resizeFramebuffer(const IntSize& size)
+bool DrawingBuffer::resizeFramebuffer(const IntSize& size, bool wantDepthOrStencilBuffer)
 {
     m_gl->BindFramebuffer(GL_FRAMEBUFFER, m_fbo);
-    if (m_antiAliasingMode != MSAAExplicitResolve)
+    if (m_antiAliasingMode != MSAAExplicitResolve && wantDepthOrStencilBuffer)

[danakj, 2016/03/31 20:27:57]

we just don't call resizeDepthStencil() instead of passing this bool just to
early out

         resizeDepthStencil(size);
     if (m_gl->CheckFramebufferStatus(GL_FRAMEBUFFER) != GL_FRAMEBUFFER_COMPLETE)
         return false;
@@ -665,7 +679,7 @@ bool DrawingBuffer::resizeFramebuffer(const IntSize& size)
     return true;
 }
 
-bool DrawingBuffer::resizeMultisampleFramebuffer(const IntSize& size)
+bool DrawingBuffer::resizeMultisampleFramebuffer(const IntSize& size, bool wantDepthOrStencilBuffer)
 {
     if (m_antiAliasingMode == MSAAExplicitResolve) {
         m_gl->BindFramebuffer(GL_FRAMEBUFFER, m_multisampleFBO);
@@ -677,7 +691,8 @@ bool DrawingBuffer::resizeMultisampleFramebuffer(const IntSize& size)
             return false;
 
         m_gl->FramebufferRenderbuffer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_RENDERBUFFER, m_multisampleColorBuffer);
-        resizeDepthStencil(size);
+        if (wantDepthOrStencilBuffer)

[danakj, 2016/03/31 20:27:57]

same here

+            resizeDepthStencil(size);
         if (m_gl->CheckFramebufferStatus(GL_FRAMEBUFFER) != GL_FRAMEBUFFER_COMPLETE)
             return false;
     }
@@ -687,9 +702,6 @@ bool DrawingBuffer::resizeMultisampleFramebuffer(const IntSize& size)
 
 void DrawingBuffer::resizeDepthStencil(const IntSize& size)
 {
-    if (!m_requestedAttributes.depth && !m_requestedAttributes.stencil)

[danakj, 2016/03/31 20:27:56]

This is the early out we don't need then

-        return;
-
     if (!m_depthStencilBuffer)
         m_gl->GenRenderbuffers(1, &m_depthStencilBuffer);
     m_gl->BindRenderbuffer(GL_RENDERBUFFER, m_depthStencilBuffer);
@@ -717,11 +729,6 @@ void DrawingBuffer::clearFramebuffers(GLbitfield clearMask)
     m_gl->Clear(clearMask);
 }
 
-bool DrawingBuffer::hasImplicitStencilBuffer() const
-{
-    return m_depthStencilBuffer && m_requestedAttributes.depth && !m_requestedAttributes.stencil;

[danakj, 2016/03/31 20:27:56]

This is stored as a bool in initialize() now

-}
-
 void DrawingBuffer::setSize(const IntSize& size)
 {
     if (m_size == size)
@@ -744,7 +751,7 @@ IntSize DrawingBuffer::adjustSize(const IntSize& desiredSize, const IntSize& cur
     return adjustedSize;
 }
 
-bool DrawingBuffer::reset(const IntSize& newSize)
+bool DrawingBuffer::reset(const IntSize& newSize, bool wantAlphaChannel, bool wantDepthOrStencilBuffer)
 {
     ASSERT(!newSize.isEmpty());
     IntSize adjustedSize = adjustSize(newSize, m_size, m_maxTextureSize);
@@ -756,14 +763,15 @@ bool DrawingBuffer::reset(const IntSize& newSize)
             if (m_colorBuffer.textureId) {
                 resizeTextureMemory(&m_colorBuffer, adjustedSize);
             } else {
-                m_colorBuffer = createTextureAndAllocateMemory(adjustedSize);
+                m_colorBuffer = createTextureAndAllocateMemory(adjustedSize, wantAlphaChannel);
             }
 
             m_gl->BindFramebuffer(GL_FRAMEBUFFER, m_fbo);
             attachColorBufferToCurrentFBO();
 
             // resize multisample FBO
-            if (!resizeMultisampleFramebuffer(adjustedSize) || !resizeFramebuffer(adjustedSize)) {
+            if (!resizeMultisampleFramebuffer(adjustedSize, wantDepthOrStencilBuffer)
+                || !resizeFramebuffer(adjustedSize, wantDepthOrStencilBuffer)) {
                 adjustedSize.scale(s_resourceAdjustedRatio);
                 continue;
             }
@@ -781,12 +789,12 @@ bool DrawingBuffer::reset(const IntSize& newSize)
     m_gl->ColorMask(true, true, true, true);
 
     GLbitfield clearMask = GL_COLOR_BUFFER_BIT;
-    if (m_actualAttributes.depth) {
+    if (m_hasDepthBuffer) {
         m_gl->ClearDepthf(1.0f);
         clearMask |= GL_DEPTH_BUFFER_BIT;
         m_gl->DepthMask(true);
     }
-    if (m_actualAttributes.stencil) {
+    if (m_hasStencilBuffer) {
         m_gl->ClearStencil(0);
         clearMask |= GL_STENCIL_BUFFER_BIT;
         m_gl->StencilMaskSeparate(GL_FRONT, 0xFFFFFFFF);
@@ -962,23 +970,23 @@ void DrawingBuffer::deleteChromiumImageForTexture(TextureInfo* info)
     }
 }
 
-DrawingBuffer::TextureInfo DrawingBuffer::createTextureAndAllocateMemory(const IntSize& size)
+DrawingBuffer::TextureInfo DrawingBuffer::createTextureAndAllocateMemory(const IntSize& size, bool wantAlphaChannel)
 {
     // TODO(erikchen): Add support for a CHROMIUM_image back buffer whose
     // behavior mimics a texture with internal format GL_RGB.
     // https://crbug.com/581777.
-    if (!m_requestedAttributes.alpha)
-        return createDefaultTextureAndAllocateMemory(size);
+    if (!wantAlphaChannel)
+        return createDefaultTextureAndAllocateMemory(size, wantAlphaChannel);
 
     if (!RuntimeEnabledFeatures::webGLImageChromiumEnabled())
-        return createDefaultTextureAndAllocateMemory(size);
+        return createDefaultTextureAndAllocateMemory(size, wantAlphaChannel);
 
     // First, try to allocate a CHROMIUM_image. This always has the potential to
     // fail.
-    TextureParameters parameters = chromiumImageTextureParameters();
+    TextureParameters parameters = chromiumImageTextureParameters(wantAlphaChannel);
     GLuint imageId = m_gl->CreateGpuMemoryBufferImageCHROMIUM(size.width(), size.height(), parameters.internalColorFormat, GC3D_SCANOUT_CHROMIUM);
     if (!imageId)
-        return createDefaultTextureAndAllocateMemory(size);
+        return createDefaultTextureAndAllocateMemory(size, wantAlphaChannel);
 
     GLuint textureId = createColorTexture(parameters);
     m_gl->BindTexImage2DCHROMIUM(parameters.target, imageId);
@@ -990,9 +998,9 @@ DrawingBuffer::TextureInfo DrawingBuffer::createTextureAndAllocateMemory(const I
     return info;
 }
 
-DrawingBuffer::TextureInfo DrawingBuffer::createDefaultTextureAndAllocateMemory(const IntSize& size)
+DrawingBuffer::TextureInfo DrawingBuffer::createDefaultTextureAndAllocateMemory(const IntSize& size, bool wantAlphaChannel)
 {
-    TextureParameters parameters = defaultTextureParameters();
+    TextureParameters parameters = defaultTextureParameters(wantAlphaChannel);
     GLuint textureId = createColorTexture(parameters);
     texImage2DResourceSafe(parameters.target, 0, parameters.internalColorFormat, size.width(), size.height(), 0, parameters.colorFormat, GL_UNSIGNED_BYTE);
 
@@ -1016,7 +1024,8 @@ void DrawingBuffer::resizeTextureMemory(TextureInfo* info, const IntSize& size)
 
         // If the desired texture target is different, there's no way to fall back
         // to a non CHROMIUM_image texture.
-        if (chromiumImageTextureParameters().target != defaultTextureParameters().target)
+        bool wantAlphaChannel = false; // Not relevant for the target.

[danakj, 2016/03/31 20:27:56]

It's a bit weird we construct the full texture params just for the target, but
whatevs. We don't need to plumb desire for alpha here to get the target.

+        if (chromiumImageTextureParameters(wantAlphaChannel).target != defaultTextureParameters(wantAlphaChannel).target)
             return;
     }