Surface_unittest.cpp

Show log
Commit

Hash : 3cfc0ce2
Author :
Date : 2025-03-24T07:41:33

Revert "Vulkan:Dont use Subject/Observer for SwapchainImageChanged"

This reverts commit 48103cb2f2b292cb50cc5a29546b358b2e47fd29.

Reason for revert: assert fails
https://ci.chromium.org/ui/p/angle/builders/ci/android-arm64-exp-test/7085/overview
I 22:27:33.697   77.533s _RunTestsOnDevice(17221FDF6000A4)  [ RUN      ] EGLAndroidAutoRefreshTest.SwapCPUThrottling/ES3_Vulkan_NoFixture
INFO:root:ERR: SurfaceVk.cpp:3165 (getCurrentFramebuffer): 	! Assert failed in getCurrentFramebuffer (../../src/libANGLE/renderer/vulkan/SurfaceVk.cpp:3165): mAcquireOperation.state == ImageAcquireState::Ready

Original change's description:
> Vulkan:Dont use Subject/Observer for SwapchainImageChanged
>
> Because we do deferred ANI (VkAcquireNextImage) call until image is
> needed, we need a way to force Context to go through
> FramebufferVk::syncState call (FramebufferVk::syncState calls
> WindowSurfaceVk::getAttachmentRenderTarget, which end up calling ANI.
> Right now we uses subject/observer mechanism, by sending
> angle::SubjectMessage::SwapchainImageChanged to all observers of
> WindowSurfaceVk. In this case it is egl::Surface. Then eglSurface
> redirects this message to its observers, which are all gl::Framebuffer's
> attachments: color, depth, stencil. Even though only color attachment
> needs to be notified, but because we don't have a separate list of
> observers, depth/stencil attachment also receive the notification and
> they early out. Then gl::Framebuffer sets
> DIRTY_BIT_COLOR_BUFFER_CONTENTS_0 dirty bit and send the
> angle::SubjectMessage::DirtyBitsFlagged to Context, which dirty DrawFBO
> and ReadFBO and dirty cached state. Note that this is specific for swap
> image changed case, there is no surface property change (surface
> property change will still trigger the subject/observer message with
> SubjectMessage::SubjectChanged message, but this occurs rarely). This
> gets worse for apps that uses multiple contexts, for the example
> pokemon_masters_ex has three contexts, each context has its own default
> frame buffer that attach to the same surface, and we never remove
> non-current context from the observer list. This end up with
> egl::Surface has 12 observers and for every frame, it loop over the list
> of 12 observers and send message (virtual function call) to each of
> them. Color attachment also ends up sending two messages to Context, one
> for Read FBO and another for Draw FBO. There are total 21 virtual
> function calls. Even for single context usage, you have 6 virtual
> function calls, for every frame.
>
> EGL spec says "an EGLSurface must be current on only one thread at a
> time", any other context must call EGLMakeCurrent in order to use this
> surface, which will add all necessary dirty bits at that time. So we
> really only need to notify current context. In this CL,
> SwapchainImageChanged no longer uses subject/observer mechanism, so this
> message is removed.
>
> This CL still uses subject/observer mechanism to send DirtyBitsFlagged
> from Framebuffer back to context. We could call setDrawFramebufferDirty
> and setReadFramebufferDirty directly, but that will require to remove
> the "const" decoration out of gl::Context which generates too much code
> diff, so onStateChange(angle::SubjectMessage::DirtyBitsFlagged) is still
> used.
>
> Bug: angleproject:400711938
> Change-Id: I61354516fd0aa307714b7abd30c6b6e45ff7b496
> Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6319893
> Commit-Queue: Charlie Lao <cclao@google.com>
> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
> Reviewed-by: Yuxin Hu <yuxinhu@google.com>

Bug: angleproject:400711938
Change-Id: Ib7899d1ac63a1f86af0953a1d25922578c470fc9
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6387755
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Yuly Novikov <ynovikov@chromium.org>

Download

src/libANGLE/Surface_unittest.cpp

//
// Copyright 2014 The ANGLE Project Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
//

#include "gmock/gmock.h"
#include "gtest/gtest.h"

#include "libANGLE/AttributeMap.h"
#include "libANGLE/Config.h"
#include "libANGLE/State.h"
#include "libANGLE/Surface.h"
#include "libANGLE/angletypes.h"
#include "libANGLE/renderer/FramebufferImpl_mock.h"
#include "libANGLE/renderer/SurfaceImpl.h"
#include "tests/angle_unittests_utils.h"

using namespace rx;
using namespace testing;

namespace
{

class MockSurfaceImpl : public rx::SurfaceImpl
{
  public:
    MockSurfaceImpl() : SurfaceImpl(mockState), mockState({1}, nullptr, egl::AttributeMap()) {}
    virtual ~MockSurfaceImpl() { destructor(); }

    MOCK_METHOD1(destroy, void(const egl::Display *));
    MOCK_METHOD1(initialize, egl::Error(const egl::Display *));
    MOCK_METHOD1(swap, egl::Error(const gl::Context *));
    MOCK_METHOD3(swapWithDamage, egl::Error(const gl::Context *, const EGLint *, EGLint));
    MOCK_METHOD5(postSubBuffer, egl::Error(const gl::Context *, EGLint, EGLint, EGLint, EGLint));
    MOCK_METHOD2(querySurfacePointerANGLE, egl::Error(EGLint, void **));
    MOCK_METHOD3(bindTexImage, egl::Error(const gl::Context *context, gl::Texture *, EGLint));
    MOCK_METHOD2(releaseTexImage, egl::Error(const gl::Context *context, EGLint));
    MOCK_METHOD3(getSyncValues, egl::Error(EGLuint64KHR *, EGLuint64KHR *, EGLuint64KHR *));
    MOCK_METHOD2(getMscRate, egl::Error(EGLint *, EGLint *));
    MOCK_METHOD2(setSwapInterval, void(const egl::Display *, EGLint));
    MOCK_CONST_METHOD0(getWidth, EGLint());
    MOCK_CONST_METHOD0(getHeight, EGLint());
    MOCK_CONST_METHOD0(isPostSubBufferSupported, EGLint(void));
    MOCK_CONST_METHOD0(getSwapBehavior, EGLint(void));
    MOCK_METHOD5(getAttachmentRenderTarget,
                 angle::Result(const gl::Context *,
                               GLenum,
                               const gl::ImageIndex &,
                               GLsizei,
                               rx::FramebufferAttachmentRenderTarget **));
    MOCK_METHOD2(attachToFramebuffer, egl::Error(const gl::Context *, gl::Framebuffer *));
    MOCK_METHOD2(detachFromFramebuffer, egl::Error(const gl::Context *, gl::Framebuffer *));
    MOCK_METHOD0(destructor, void());

    egl::SurfaceState mockState;
};

TEST(SurfaceTest, DestructionDeletesImpl)
{
    NiceMock<MockEGLFactory> factory;

    MockSurfaceImpl *impl = new MockSurfaceImpl;
    EXPECT_CALL(factory, createWindowSurface(_, _, _)).WillOnce(Return(impl));

    egl::Config config;
    egl::Surface *surface = new egl::WindowSurface(
        &factory, {1}, &config, static_cast<EGLNativeWindowType>(0), egl::AttributeMap(), false);

    EXPECT_CALL(*impl, destroy(_)).Times(1).RetiresOnSaturation();
    EXPECT_CALL(*impl, destructor()).Times(1).RetiresOnSaturation();

    EXPECT_FALSE(surface->onDestroy(nullptr).isError());

    // Only needed because the mock is leaked if bugs are present,
    // which logs an error, but does not cause the test to fail.
    // Ordinarily mocks are verified when destroyed.
    Mock::VerifyAndClear(impl);
}

}  // namespace

Download