Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit a1742966 authored by Tom Murphy's avatar Tom Murphy
Browse files

Build egl_display extension string using temporary vector 

Currently it is very easy to forget to add a trailing space when adding to the extension string. This has already caused a major bug fixed by 0bc64a80.

Build the string using a vector instead similar to libANGLE::Context::initExtensionStrings() does it. This is much less error prone than the current code.

Test: Added unit test
Test: Ran atest EGL_test
Test: Confirmed string output is the same with print
Bug: 285606242
Change-Id: I7f837a3009f0ccfedcf44f3d7ff709a269d0034f
parent 5f5998f2

opengl/libs/EGL/egl_display.cpp

+23 −12
Original line number Original line Diff line number Diff line
@@ -254,6 +254,7 @@ EGLBoolean egl_display_t::initialize(EGLint* major, EGLint* minor) { 
        }

        }

    }

    }





    std::vector<std::string> extensionStrings;

    { // scope for lock

    { // scope for lock

        std::lock_guard<std::mutex> _l(lock);

        std::lock_guard<std::mutex> _l(lock);
@@ -315,16 +316,14 @@ EGLBoolean egl_display_t::initialize(EGLint* major, EGLint* minor) { 
        }

        }

        mClientApiString = sClientApiString;

        mClientApiString = sClientApiString;





        mExtensionString = gBuiltinExtensionString;



        // b/269060366 Conditionally enabled EGL_ANDROID_get_frame_timestamps extension if the

        // b/269060366 Conditionally enabled EGL_ANDROID_get_frame_timestamps extension if the

        // device's present timestamps are reliable (which may not be the case on emulators).

        // device's present timestamps are reliable (which may not be the case on emulators).

        if (cnx->angleLoaded) {

        if (cnx->angleLoaded) {

            if (android::base::GetBoolProperty("service.sf.present_timestamp", false)) {

            if (android::base::GetBoolProperty("service.sf.present_timestamp", false)) {

                mExtensionString.append("EGL_ANDROID_get_frame_timestamps ");

                extensionStrings.push_back("EGL_ANDROID_get_frame_timestamps");

            }

            }

        } else {

        } else {

            mExtensionString.append("EGL_ANDROID_get_frame_timestamps ");

            extensionStrings.push_back("EGL_ANDROID_get_frame_timestamps");

        }

        }





        hasColorSpaceSupport = findExtension(disp.queryString.extensions, "EGL_KHR_gl_colorspace");

        hasColorSpaceSupport = findExtension(disp.queryString.extensions, "EGL_KHR_gl_colorspace");
@@ -335,10 +334,12 @@ EGLBoolean egl_display_t::initialize(EGLint* major, EGLint* minor) { 




        // Add wide-color extensions if device can support wide-color

        // Add wide-color extensions if device can support wide-color

        if (wideColorBoardConfig && hasColorSpaceSupport) {

        if (wideColorBoardConfig && hasColorSpaceSupport) {

            mExtensionString.append(

            std::vector<std::string> wideColorExtensions =

                    "EGL_EXT_gl_colorspace_scrgb EGL_EXT_gl_colorspace_scrgb_linear "

                    {"EGL_EXT_gl_colorspace_scrgb", "EGL_EXT_gl_colorspace_scrgb_linear",

                    "EGL_EXT_gl_colorspace_display_p3_linear EGL_EXT_gl_colorspace_display_p3 "

                     "EGL_EXT_gl_colorspace_display_p3_linear", "EGL_EXT_gl_colorspace_display_p3",

                    "EGL_EXT_gl_colorspace_display_p3_passthrough ");

                     "EGL_EXT_gl_colorspace_display_p3_passthrough"};

            extensionStrings.insert(extensionStrings.end(), wideColorExtensions.begin(),

                                    wideColorExtensions.end());

        }

        }





        bool hasHdrBoardConfig = android::sysprop::has_HDR_display(false);

        bool hasHdrBoardConfig = android::sysprop::has_HDR_display(false);
@@ -348,9 +349,11 @@ EGLBoolean egl_display_t::initialize(EGLint* major, EGLint* minor) { 
            // Typically that means there is an HDR capable display attached, but could be

            // Typically that means there is an HDR capable display attached, but could be

            // support for attaching an HDR display. In either case, advertise support for

            // support for attaching an HDR display. In either case, advertise support for

            // HDR color spaces.

            // HDR color spaces.

            mExtensionString.append("EGL_EXT_gl_colorspace_bt2020_hlg "

            std::vector<std::string> hdrExtensions = {"EGL_EXT_gl_colorspace_bt2020_hlg",

                                    "EGL_EXT_gl_colorspace_bt2020_linear "

                                                      "EGL_EXT_gl_colorspace_bt2020_linear",

                                    "EGL_EXT_gl_colorspace_bt2020_pq ");

                                                      "EGL_EXT_gl_colorspace_bt2020_pq"};

            extensionStrings.insert(extensionStrings.end(), hdrExtensions.begin(),

                                    hdrExtensions.end());

        }

        }





        char const* start = gExtensionString;

        char const* start = gExtensionString;
@@ -361,7 +364,7 @@ EGLBoolean egl_display_t::initialize(EGLint* major, EGLint* minor) { 
                // NOTE: we could avoid the copy if we had strnstr.

                // NOTE: we could avoid the copy if we had strnstr.

                const std::string ext(start, len);

                const std::string ext(start, len);

                if (findExtension(disp.queryString.extensions, ext.c_str(), len)) {

                if (findExtension(disp.queryString.extensions, ext.c_str(), len)) {

                    mExtensionString.append(ext + " ");

                    extensionStrings.push_back(ext);

                }

                }

                // advance to the next extension name, skipping the space.

                // advance to the next extension name, skipping the space.

                start += len;

                start += len;
@@ -388,6 +391,14 @@ EGLBoolean egl_display_t::initialize(EGLint* major, EGLint* minor) { 
        refCond.notify_all();

        refCond.notify_all();

    }

    }





    auto mergeExtensionStrings = [](const std::vector<std::string>& strings) {

        std::ostringstream combinedStringStream;

        std::copy(strings.begin(), strings.end(),

                  std::ostream_iterator<std::string>(combinedStringStream, " "));

        // gBuiltinExtensionString already has a trailing space so is added here

        return gBuiltinExtensionString + combinedStringStream.str();

    };

    mExtensionString = mergeExtensionStrings(extensionStrings);

    return EGL_TRUE;

    return EGL_TRUE;

}

}

opengl/tests/EGLTest/Android.bp

+5 −0
Original line number Original line Diff line number Diff line
@@ -37,6 +37,11 @@ cc_test { 
        "libSurfaceFlingerProp",

        "libSurfaceFlingerProp",

    ],

    ],





    static_libs: [

        "libgmock",

        "libgtest",

    ],



    include_dirs: [

    include_dirs: [

        "frameworks/native/opengl/libs",

        "frameworks/native/opengl/libs",

        "frameworks/native/opengl/libs/EGL",

        "frameworks/native/opengl/libs/EGL",

opengl/tests/EGLTest/EGL_test.cpp

+56 −0
Original line number Original line Diff line number Diff line
@@ -15,6 +15,7 @@ 
 */

 */





#include <gtest/gtest.h>

#include <gtest/gtest.h>

#include <gmock/gmock.h>





#include <SurfaceFlingerProperties.h>

#include <SurfaceFlingerProperties.h>

#include <android/hardware/configstore/1.0/ISurfaceFlingerConfigs.h>

#include <android/hardware/configstore/1.0/ISurfaceFlingerConfigs.h>
@@ -29,6 +30,8 @@ 
#include <gui/IGraphicBufferConsumer.h>

#include <gui/IGraphicBufferConsumer.h>

#include <gui/BufferQueue.h>

#include <gui/BufferQueue.h>





#include "egl_display.h"



bool hasEglExtension(EGLDisplay dpy, const char* extensionName) {

bool hasEglExtension(EGLDisplay dpy, const char* extensionName) {

    const char* exts = eglQueryString(dpy, EGL_EXTENSIONS);

    const char* exts = eglQueryString(dpy, EGL_EXTENSIONS);

    size_t cropExtLen = strlen(extensionName);

    size_t cropExtLen = strlen(extensionName);
@@ -1011,4 +1014,57 @@ TEST_F(EGLTest, EGLCreateWindowTwoColorspaces) { 




    EXPECT_TRUE(eglDestroySurface(mEglDisplay, eglSurface));

    EXPECT_TRUE(eglDestroySurface(mEglDisplay, eglSurface));

}

}



TEST_F(EGLTest, EGLCheckExtensionString) {

    // check that the format of the extension string is correct



    egl_display_t* display = egl_display_t::get(mEglDisplay);

    ASSERT_NE(display, nullptr);



    std::string extensionStrRegex = "((EGL_ANDROID_front_buffer_auto_refresh|"

       "EGL_ANDROID_get_native_client_buffer|"

       "EGL_ANDROID_presentation_time|"

       "EGL_EXT_surface_CTA861_3_metadata|"

       "EGL_EXT_surface_SMPTE2086_metadata|"

       "EGL_KHR_get_all_proc_addresses|"

       "EGL_KHR_swap_buffers_with_damage|"

       "EGL_ANDROID_get_frame_timestamps|"

       "EGL_EXT_gl_colorspace_scrgb|"

       "EGL_EXT_gl_colorspace_scrgb_linear|"

       "EGL_EXT_gl_colorspace_display_p3_linear|"

       "EGL_EXT_gl_colorspace_display_p3|"

       "EGL_EXT_gl_colorspace_display_p3_passthrough|"

       "EGL_EXT_gl_colorspace_bt2020_hlg|"

       "EGL_EXT_gl_colorspace_bt2020_linear|"

       "EGL_EXT_gl_colorspace_bt2020_pq|"

       "EGL_ANDROID_image_native_buffer|"

       "EGL_ANDROID_native_fence_sync|"

       "EGL_ANDROID_recordable|"

       "EGL_EXT_create_context_robustness|"

       "EGL_EXT_image_gl_colorspace|"

       "EGL_EXT_pixel_format_float|"

       "EGL_EXT_protected_content|"

       "EGL_EXT_yuv_surface|"

       "EGL_IMG_context_priority|"

       "EGL_KHR_config_attribs|"

       "EGL_KHR_create_context|"

       "EGL_KHR_fence_sync|"

       "EGL_KHR_gl_colorspace|"

       "EGL_KHR_gl_renderbuffer_image|"

       "EGL_KHR_gl_texture_2D_image|"

       "EGL_KHR_gl_texture_3D_image|"

       "EGL_KHR_gl_texture_cubemap_image|"

       "EGL_KHR_image|"

       "EGL_KHR_image_base|"

       "EGL_KHR_mutable_render_buffer|"

       "EGL_KHR_no_config_context|"

       "EGL_KHR_partial_update|"

       "EGL_KHR_surfaceless_context|"

       "EGL_KHR_wait_sync|"

       "EGL_EXT_buffer_age|"

       "EGL_KHR_reusable_sync|"

       "EGL_NV_context_priority_realtime) )+";

    EXPECT_THAT(display->getExtensionString(), testing::MatchesRegex(extensionStrRegex));

}



}
 
}