[Libreoffice-commits] core.git: vcl/inc vcl/opengl vcl/qa vcl/skia vcl/source
Luboš Luňák (via logerrit)
logerrit at kemper.freedesktop.org
Tue Sep 22 12:59:24 UTC 2020
vcl/inc/driverblocklist.hxx | 28 ++++++++++++++--------
vcl/opengl/win/WinDeviceInfo.cxx | 3 +-
vcl/qa/cppunit/blocklistparsertest.cxx | 26 +++++++++++---------
vcl/skia/SkiaHelper.cxx | 5 ++--
vcl/source/helper/driverblocklist.cxx | 41 ++++++++++++++++++++-------------
5 files changed, 62 insertions(+), 41 deletions(-)
New commits:
commit bf62a89030aeb61cfdc032b167bc62ce6a3240dc
Author: Luboš Luňák <l.lunak at centrum.cz>
AuthorDate: Mon Sep 21 14:44:32 2020 +0000
Commit: Luboš Luňák <l.lunak at collabora.com>
CommitDate: Tue Sep 22 14:58:41 2020 +0200
fix parsing of Vulkan version numbers
The Windows+OpenGL handling pads the numbers so that .98 > 0.978,
but Vulkan uses "normal" numeric ordering for versions.
Change-Id: I766baf50e3505a96aa85163962400bb69c0acea6
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/103118
Tested-by: Jenkins
Reviewed-by: Luboš Luňák <l.lunak at collabora.com>
diff --git a/vcl/inc/driverblocklist.hxx b/vcl/inc/driverblocklist.hxx
index a417d19ff543..2e1367e5db31 100644
--- a/vcl/inc/driverblocklist.hxx
+++ b/vcl/inc/driverblocklist.hxx
@@ -18,8 +18,16 @@
namespace DriverBlocklist
{
-VCL_DLLPUBLIC bool IsDeviceBlocked(const OUString& blocklistURL, const OUString& driverVersion,
- const OUString& vendorId, const OUString& deviceId);
+// Details of how to treat a version number.
+enum class VersionType
+{
+ OpenGL, // a.b.c.d, 1.98 > 1.978
+ Vulkan // a.b.c , 1.98 < 1.978
+};
+
+VCL_DLLPUBLIC bool IsDeviceBlocked(const OUString& blocklistURL, VersionType versionType,
+ const OUString& driverVersion, const OUString& vendorId,
+ const OUString& deviceId);
#ifdef _WIN32
VCL_DLLPUBLIC int32_t GetWindowsVersion();
@@ -103,7 +111,7 @@ struct DriverInfo
class VCL_DLLPUBLIC Parser
{
public:
- Parser(const OUString& rURL, std::vector<DriverInfo>& rDriverList);
+ Parser(const OUString& rURL, std::vector<DriverInfo>& rDriverList, VersionType versionType);
bool parse();
private:
@@ -111,6 +119,7 @@ private:
void handleList(xmlreader::XmlReader& rReader);
void handleContent(xmlreader::XmlReader& rReader);
static void handleDevices(DriverInfo& rDriver, xmlreader::XmlReader& rReader);
+ uint64_t getVersion(const OString& rString);
enum class BlockType
{
@@ -122,21 +131,20 @@ private:
BlockType meBlockType;
std::vector<DriverInfo>& mrDriverList;
OUString maURL;
+ const VersionType mVersionType;
};
OUString VCL_DLLPUBLIC GetVendorId(DeviceVendor id);
-bool VCL_DLLPUBLIC FindBlocklistedDeviceInList(std::vector<DriverInfo>& aDeviceInfos,
- OUString const& sDriverVersion,
- OUString const& sAdapterVendorID,
- OUString const& sAdapterDeviceID,
- OperatingSystem system,
- const OUString& blocklistURL = OUString());
+bool VCL_DLLPUBLIC FindBlocklistedDeviceInList(
+ std::vector<DriverInfo>& aDeviceInfos, VersionType versionType, OUString const& sDriverVersion,
+ OUString const& sAdapterVendorID, OUString const& sAdapterDeviceID, OperatingSystem system,
+ const OUString& blocklistURL = OUString());
#define GFX_DRIVER_VERSION(a, b, c, d) \
((uint64_t(a) << 48) | (uint64_t(b) << 32) | (uint64_t(c) << 16) | uint64_t(d))
-inline uint64_t V(uint32_t a, uint32_t b, uint32_t c, uint32_t d)
+inline uint64_t OpenGLVersion(uint32_t a, uint32_t b, uint32_t c, uint32_t d)
{
// We make sure every driver number is padded by 0s, this will allow us the
// easiest 'compare as if decimals' approach. See ParseDriverVersion for a
diff --git a/vcl/opengl/win/WinDeviceInfo.cxx b/vcl/opengl/win/WinDeviceInfo.cxx
index 9d95fc0be272..c936aee11fa6 100644
--- a/vcl/opengl/win/WinDeviceInfo.cxx
+++ b/vcl/opengl/win/WinDeviceInfo.cxx
@@ -176,7 +176,8 @@ static OUString getDenylistFile()
bool WinOpenGLDeviceInfo::FindBlocklistedDeviceInList()
{
- return DriverBlocklist::IsDeviceBlocked( getDenylistFile(), maDriverVersion, maAdapterVendorID, maAdapterDeviceID);
+ return DriverBlocklist::IsDeviceBlocked( getDenylistFile(), DriverBlocklist::VersionType::OpenGL,
+ maDriverVersion, maAdapterVendorID, maAdapterDeviceID);
}
namespace {
diff --git a/vcl/qa/cppunit/blocklistparsertest.cxx b/vcl/qa/cppunit/blocklistparsertest.cxx
index b79de071d319..540cbe436887 100644
--- a/vcl/qa/cppunit/blocklistparsertest.cxx
+++ b/vcl/qa/cppunit/blocklistparsertest.cxx
@@ -38,7 +38,8 @@ void BlocklistParserTest::testParse()
{
std::vector<DriverInfo> aDriveInfos;
- Parser aBlocklistParser(m_directories.getURLFromSrc("vcl/qa/cppunit/") + "test_blocklist_parse.xml", aDriveInfos);
+ Parser aBlocklistParser(m_directories.getURLFromSrc("vcl/qa/cppunit/") + "test_blocklist_parse.xml",
+ aDriveInfos, VersionType::OpenGL);
aBlocklistParser.parse();
size_t const n = aDriveInfos.size();
@@ -52,7 +53,7 @@ void BlocklistParserTest::testParse()
CPPUNIT_ASSERT_EQUAL(bIsAllowlisted, aDriveInfo.mbAllowlisted);
CPPUNIT_ASSERT_EQUAL(GetVendorId(VendorAll), aDriveInfo.maAdapterVendor); // "all"
CPPUNIT_ASSERT_EQUAL(VersionComparisonOp::DRIVER_LESS_THAN, aDriveInfo.meComparisonOp);
- CPPUNIT_ASSERT_EQUAL(V(10,20,30,40), aDriveInfo.mnDriverVersion);
+ CPPUNIT_ASSERT_EQUAL(OpenGLVersion(10,20,30,40), aDriveInfo.mnDriverVersion);
aDriveInfo = aDriveInfos[i++];
CPPUNIT_ASSERT_EQUAL(bIsAllowlisted, aDriveInfo.mbAllowlisted);
@@ -95,7 +96,8 @@ void BlocklistParserTest::testEvaluate()
{
std::vector<DriverInfo> aDriveInfos;
- Parser aBlocklistParser(m_directories.getURLFromSrc("vcl/qa/cppunit/") + "test_blocklist_evaluate.xml", aDriveInfos);
+ Parser aBlocklistParser(m_directories.getURLFromSrc("vcl/qa/cppunit/") + "test_blocklist_evaluate.xml",
+ aDriveInfos, VersionType::OpenGL);
aBlocklistParser.parse();
OUString vendorAMD = GetVendorId(VendorAMD);
@@ -105,30 +107,30 @@ void BlocklistParserTest::testEvaluate()
// Check OS
CPPUNIT_ASSERT_EQUAL(false, FindBlocklistedDeviceInList(
- aDriveInfos, "10.20.30.40", vendorNVIDIA, "all", DRIVER_OS_WINDOWS_7));
+ aDriveInfos, VersionType::OpenGL, "10.20.30.40", vendorNVIDIA, "all", DRIVER_OS_WINDOWS_7));
CPPUNIT_ASSERT_EQUAL(false, FindBlocklistedDeviceInList(
- aDriveInfos, "10.20.30.40", vendorNVIDIA, "all", DRIVER_OS_WINDOWS_8));
+ aDriveInfos, VersionType::OpenGL, "10.20.30.40", vendorNVIDIA, "all", DRIVER_OS_WINDOWS_8));
CPPUNIT_ASSERT_EQUAL(false, FindBlocklistedDeviceInList(
- aDriveInfos, "10.20.30.40", vendorNVIDIA, "all", DRIVER_OS_WINDOWS_10));
+ aDriveInfos, VersionType::OpenGL, "10.20.30.40", vendorNVIDIA, "all", DRIVER_OS_WINDOWS_10));
// Check Vendors
CPPUNIT_ASSERT_EQUAL(true, FindBlocklistedDeviceInList(
- aDriveInfos, "10.20.30.40", vendorMicrosoft, "all", DRIVER_OS_WINDOWS_7));
+ aDriveInfos, VersionType::OpenGL, "10.20.30.40", vendorMicrosoft, "all", DRIVER_OS_WINDOWS_7));
CPPUNIT_ASSERT_EQUAL(true, FindBlocklistedDeviceInList(
- aDriveInfos, "10.20.30.40", vendorMicrosoft, "all", DRIVER_OS_WINDOWS_10));
+ aDriveInfos, VersionType::OpenGL, "10.20.30.40", vendorMicrosoft, "all", DRIVER_OS_WINDOWS_10));
// Check Versions
CPPUNIT_ASSERT_EQUAL(true, FindBlocklistedDeviceInList(
- aDriveInfos, "10.20.30.39", vendorAMD, "all", DRIVER_OS_WINDOWS_7));
+ aDriveInfos, VersionType::OpenGL, "10.20.30.39", vendorAMD, "all", DRIVER_OS_WINDOWS_7));
CPPUNIT_ASSERT_EQUAL(false, FindBlocklistedDeviceInList(
- aDriveInfos, "10.20.30.40", vendorAMD, "all", DRIVER_OS_WINDOWS_7));
+ aDriveInfos, VersionType::OpenGL, "10.20.30.40", vendorAMD, "all", DRIVER_OS_WINDOWS_7));
CPPUNIT_ASSERT_EQUAL(false, FindBlocklistedDeviceInList(
- aDriveInfos, "10.20.30.41", vendorAMD, "all", DRIVER_OS_WINDOWS_7));
+ aDriveInfos, VersionType::OpenGL, "10.20.30.41", vendorAMD, "all", DRIVER_OS_WINDOWS_7));
// Check
CPPUNIT_ASSERT_EQUAL(true, FindBlocklistedDeviceInList(
- aDriveInfos, "9.17.10.4229", vendorIntel, "all", DRIVER_OS_WINDOWS_7));
+ aDriveInfos, VersionType::OpenGL, "9.17.10.4229", vendorIntel, "all", DRIVER_OS_WINDOWS_7));
}
diff --git a/vcl/skia/SkiaHelper.cxx b/vcl/skia/SkiaHelper.cxx
index 5bf8c04cff6a..72944a946c71 100644
--- a/vcl/skia/SkiaHelper.cxx
+++ b/vcl/skia/SkiaHelper.cxx
@@ -128,8 +128,9 @@ static bool isVulkanDenylisted(const VkPhysicalDeviceProperties& props)
<< ", vendor: " << vendorIdStr << " ("
<< vendorAsString(vendorId) << "), device: " << deviceIdStr
<< ", type: " << deviceType << ", name: " << props.deviceName);
- bool denylisted = DriverBlocklist::IsDeviceBlocked(getDenylistFile(), driverVersionString,
- vendorIdStr, deviceIdStr);
+ bool denylisted
+ = DriverBlocklist::IsDeviceBlocked(getDenylistFile(), DriverBlocklist::VersionType::Vulkan,
+ driverVersionString, vendorIdStr, deviceIdStr);
writeToLog(logFile, "Denylisted", denylisted ? "yes" : "no");
return denylisted;
}
diff --git a/vcl/source/helper/driverblocklist.cxx b/vcl/source/helper/driverblocklist.cxx
index 6bcee982015f..ea153eb1fc8c 100644
--- a/vcl/source/helper/driverblocklist.cxx
+++ b/vcl/source/helper/driverblocklist.cxx
@@ -174,10 +174,11 @@ std::string_view GetVendorNameFromId(uint32_t id)
}
}
-Parser::Parser(const OUString& rURL, std::vector<DriverInfo>& rDriverList)
+Parser::Parser(const OUString& rURL, std::vector<DriverInfo>& rDriverList, VersionType versionType)
: meBlockType(BlockType::UNKNOWN)
, mrDriverList(rDriverList)
, maURL(rURL)
+ , mVersionType(versionType)
{
}
@@ -219,7 +220,7 @@ static void PadDriverDecimal(char* aString)
// All destination string storage needs to have at least 5 bytes available.
static bool SplitDriverVersion(const char* aSource, char* aAStr, char* aBStr, char* aCStr,
- char* aDStr)
+ char* aDStr, VersionType versionType)
{
// sscanf doesn't do what we want here to we parse this manually.
int len = strlen(aSource);
@@ -256,7 +257,7 @@ static bool SplitDriverVersion(const char* aSource, char* aAStr, char* aBStr, ch
dest[destIdx][destPos] = 0;
// Vulkan version numbers have only 3 fields.
- if (destIdx == SAL_N_ELEMENTS(dest) - 2)
+ if (versionType == VersionType::Vulkan && destIdx == SAL_N_ELEMENTS(dest) - 2)
dest[++destIdx][0] = '\0';
if (destIdx != SAL_N_ELEMENTS(dest) - 1)
{
@@ -265,7 +266,8 @@ static bool SplitDriverVersion(const char* aSource, char* aAStr, char* aBStr, ch
return true;
}
-static bool ParseDriverVersion(const OUString& aVersion, uint64_t& rNumericVersion)
+static bool ParseDriverVersion(const OUString& aVersion, uint64_t& rNumericVersion,
+ VersionType versionType)
{
rNumericVersion = 0;
@@ -273,18 +275,24 @@ static bool ParseDriverVersion(const OUString& aVersion, uint64_t& rNumericVersi
char aStr[8], bStr[8], cStr[8], dStr[8];
/* honestly, why do I even bother */
OString aOVersion = OUStringToOString(aVersion, RTL_TEXTENCODING_UTF8);
- if (!SplitDriverVersion(aOVersion.getStr(), aStr, bStr, cStr, dStr))
+ if (!SplitDriverVersion(aOVersion.getStr(), aStr, bStr, cStr, dStr, versionType))
return false;
- PadDriverDecimal(bStr);
- PadDriverDecimal(cStr);
- PadDriverDecimal(dStr);
+ if (versionType == VersionType::OpenGL)
+ {
+ PadDriverDecimal(bStr);
+ PadDriverDecimal(cStr);
+ PadDriverDecimal(dStr);
+ }
a = atoi(aStr);
b = atoi(bStr);
c = atoi(cStr);
d = atoi(dStr);
+ if (versionType == VersionType::Vulkan)
+ assert(d == 0);
+
if (a < 0 || a > 0xffff)
return false;
if (b < 0 || b > 0xffff)
@@ -298,11 +306,11 @@ static bool ParseDriverVersion(const OUString& aVersion, uint64_t& rNumericVersi
return true;
}
-static uint64_t getVersion(const OString& rString)
+uint64_t Parser::getVersion(const OString& rString)
{
OUString aString = OStringToOUString(rString, RTL_TEXTENCODING_UTF8);
uint64_t nVersion;
- bool bResult = ParseDriverVersion(aString, nVersion);
+ bool bResult = ParseDriverVersion(aString, nVersion, mVersionType);
if (!bResult)
{
@@ -583,13 +591,13 @@ DriverInfo::DriverInfo(OperatingSystem os, const OUString& vendor, VersionCompar
maSuggestedVersion = OStringToOUString(OString(suggestedVersion), RTL_TEXTENCODING_UTF8);
}
-bool FindBlocklistedDeviceInList(std::vector<DriverInfo>& aDeviceInfos,
+bool FindBlocklistedDeviceInList(std::vector<DriverInfo>& aDeviceInfos, VersionType versionType,
OUString const& sDriverVersion, OUString const& sAdapterVendorID,
OUString const& sAdapterDeviceID, OperatingSystem system,
const OUString& blocklistURL)
{
uint64_t driverVersion;
- ParseDriverVersion(sDriverVersion, driverVersion);
+ ParseDriverVersion(sDriverVersion, driverVersion, versionType);
bool match = false;
for (std::vector<DriverInfo>::size_type i = 0; i < aDeviceInfos.size(); i++)
@@ -677,17 +685,18 @@ bool FindBlocklistedDeviceInList(std::vector<DriverInfo>& aDeviceInfos,
return match;
}
-bool IsDeviceBlocked(const OUString& blocklistURL, const OUString& driverVersion,
- const OUString& vendorId, const OUString& deviceId)
+bool IsDeviceBlocked(const OUString& blocklistURL, VersionType versionType,
+ const OUString& driverVersion, const OUString& vendorId,
+ const OUString& deviceId)
{
std::vector<DriverInfo> driverList;
- Parser parser(blocklistURL, driverList);
+ Parser parser(blocklistURL, driverList, versionType);
if (!parser.parse())
{
SAL_WARN("vcl.driver", "error parsing denylist " << blocklistURL);
return false;
}
- return FindBlocklistedDeviceInList(driverList, driverVersion, vendorId, deviceId,
+ return FindBlocklistedDeviceInList(driverList, versionType, driverVersion, vendorId, deviceId,
getOperatingSystem(), blocklistURL);
}
More information about the Libreoffice-commits
mailing list