widelands-dev team mailing list archive

[Notabilis <notabilis27@xxxxxx>]

Review: Needs Fixing diff

Some comments to your comments in the old diff.
Also, the new diff is broken and contains the remains of a failed merge.

Diff comments:

> === modified file 'src/graphic/gl/initialize.cc'
> --- src/graphic/gl/initialize.cc	2018-10-15 05:26:10 +0000
> +++ src/graphic/gl/initialize.cc	2018-10-15 06:50:28 +0000
> @@ -178,11 +179,26 @@
>  	glGetIntegerv(GL_MAX_TEXTURE_SIZE, max_texture_size);
>  	log("Graphics: OpenGL: Max texture size: %u\n", *max_texture_size);
>  
> -	// TODO(GunChleoc): Localize the on-screen error messages
> -	// Exit if we can't detect the shading language version
>  	const char* const shading_language_version_string =
>  	   reinterpret_cast<const char*>(glGetString(GL_SHADING_LANGUAGE_VERSION));
> -	if (strcmp(shading_language_version_string, "(null)") == 0) {
> +	log("Graphics: OpenGL: ShadingLanguage: \"%s\"\n", shading_language_version_string);

Current trunk first checks for "(null)", and only if it is not "(null)" prints out the current ShadingLanguage string. The new code always prints the string, independent of its contents.

> +
> +	std::vector<std::string> shading_language_version_vector;
> +	boost::split(shading_language_version_vector, shading_language_version_string, boost::is_any_of("."));
> +	if (shading_language_version_vector.size() >= 2) {

Sorry, should have written this in two comments. "1.20.1" is a good argument, keep this line it as ">= 2".

If "2" is reported as shading language by the driver (instead of "2.0"), shading_language_version_vector.size() will only be 1. So we would decline the version even when actually "2" > "1.20".
When I think about it: My comment regarding version "1.2" is most likely wrong and won't be a problem, sorry.

> +		// The shading language version has been detected properly. Exit if the shading language version is too old.
> +		const int major_shading_language_version = atoi(shading_language_version_vector.front().c_str());
> +		const int minor_shading_language_version = atoi(shading_language_version_vector.at(1).c_str());
> +		if (major_shading_language_version < 1 || (major_shading_language_version == 1 && minor_shading_language_version < 20)) {
> +			log("ERROR: Shading language version is too old!\n");
> +			SDL_ShowSimpleMessageBox(SDL_MESSAGEBOX_ERROR, "OpenGL Error",
> +									 "Widelands won’t work because your graphics driver is too old.\nThe "
> +									 "Shading language needs to be version 1.20 or newer.",
> +									 NULL);
> +			exit(1);
> +		}
> +	} else {
> +		// Exit because we couldn't detect the shading language version, so there must be a problem communicating with the graphics adapter.
>  		log("ERROR: Unable to detect the shading language version!\n");
>  		SDL_ShowSimpleMessageBox(SDL_MESSAGEBOX_ERROR, "OpenGL Error",
>  		                         "Widelands won't work because we were unable to detect the shading "


-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1797792-shading-language-version-comparison/+merge/356699
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1797792-shading-language-version-comparison.