In D4969#211913, @Stan wrote:

The Readme should probably mention you need to install GLSLC (does it have a minimum version?)

Added.

In D4966#211595, @Stan wrote:

In the future we need to replace all textures by CLAMP_TO_EDGE, because border color might be only black, white or transparent on some hardware.

Can you clarify this for me? Do you mean texture samplers? What is the border here?

I mean address mode of texture samplers, particularly Sampler::AddressMode::CLAMP_TO_EDGE.

In D4949#211376, @phosit wrote:

It already is accessable.

Nope, it's implementation defined. You just recieve const JS::RootedValue& librarySettings. It might be silently built or copied inside.

In D4949#211374, @phosit wrote:

That isn't that hard: make m_LibrarySettings public and use reporter.m_LibrarySettings.

It's not good to allow access for everyone.

In D4949#211371, @phosit wrote:

Whow that changed much... Im not sure if it's for the better. I dont like the callback (it hides the data-flow)

appendLibrary(LibraryReporter{rq, "boost"}.Add("version", BOOST_VERSION));

In appendLibrary you still need to extract JS::RootedValue (JS::RootedValue isn't movable nor copyable IIRC) that means you still hides the data-flow.

In D4949#211105, @phosit wrote:

I would make libraries an array instead of a class (I know it's not called class in JS).
Should "libraries" only contain versions then it should be called "library_versions". If it does contain everithing library related (I prefere that option) then SDL should also be in there.

I don't mind, though it'll make the report a bit bigger:

// This:
{
	"enet": "...",
	// ...
}
// Or this:
[
	{
		"name": "enet",
		"version": "...",
	},
	// ...
]

Propably also Vulkan...

Vulkan isn't just a library, it's a backend which depends on environment and configuration. Libraries look more constant in that way.

In D4949#211029, @phosit wrote:

Why not call ReportSDL from within ReportLibraries?

I was thinking about that. I'm not sure which variant is better. In one hand SDL is a library on other hand SDL needs to print not only its version but also some additional info like CPU count.

I'd prefer to rewrite our event system, because currently it's too C-style and unsafe.

In D4927#210297, @phosit wrote:

Is that a consern or an idea for another task?
Why is it less error prone?
Nit: It would add duplication.

I'd prefer to calculate the number of draw calls and related numbers inside backend as it's much less error prone.

In rP27490#60081, @Itms wrote:

so I'm a bit bothered and I'd like opinions. I'd go with the first fix unless you prefer the second one.

I don't mind both of them, but upstream should be done and shouldn't be forgotten.

It seems ok for Atlas.

In D4917#209972, @phosit wrote:

BTW: did you see the warnings ^ on gcc? I have the same.

Yeah, I need to update VMA.

Looks ok for Atlas.

I got the following warnings when build with --build-shared-glooxwrapper:

libraries\win32\gloox\include\gloox\clientbase.h(769): warning C4373: 'gloox::ClientBase::handlePing': virtual function overrides 'gloox::PingHandler::handlePing', previous versions of the compiler did not override when parameters only differed by const/volatile qualifiers
libraries\win32\gloox\include\gloox\pinghandler.h(61): note: see declaration of 'gloox::PingHandler::handlePing'
libraries\win32\gloox\include\gloox\clientbase.h(769): warning C4373: 'gloox::ClientBase::handlePing': virtual function overrides 'gloox::PingHandler::handlePing', previous versions of the compiler did not override when parameters only differed by const/volatile qualifiers (compiling source file ..\..\..\source\lobby\StanzaExtensions.cpp)
libraries\win32\gloox\include\gloox\pinghandler.h(61): note: see declaration of 'gloox::PingHandler::handlePing' (compiling source file ..\..\..\source\lobby\StanzaExtensions.cpp)
libraries\win32\gloox\include\gloox\clientbase.h(769): warning C4373: 'gloox::ClientBase::handlePing': virtual function overrides 'gloox::PingHandler::handlePing', previous versions of the compiler did not override when parameters only differed by const/volatile qualifiers (compiling source file ..\..\..\source\lobby\scripting\GlooxScriptConversions.cpp)
libraries\win32\gloox\include\gloox\pinghandler.h(61): note: see declaration of 'gloox::PingHandler::handlePing' (compiling source file ..\..\..\source\lobby\scripting\GlooxScriptConversions.cpp)
libraries\win32\gloox\include\gloox\clientbase.h(769): warning C4373: 'gloox::ClientBase::handlePing': virtual function overrides 'gloox::PingHandler::handlePing', previous versions of the compiler did not override when parameters only differed by const/volatile qualifiers (compiling source file ..\..\..\source\lobby\XmppClient.cpp)
libraries\win32\gloox\include\gloox\pinghandler.h(61): note: see declaration of 'gloox::PingHandler::handlePing' (compiling source file ..\..\..\source\lobby\XmppClient.cpp)

In D4905#209775, @Stan wrote:

Thanks for the detailed reports:

• Vulkan doesn't support vsync yet.

Vulkan does support vsync, MoltenVK might not.