The patch looks good to me, but I haven't tested it.

Replaces gl_FragData by gl_FragColor and removing additional draw buffer.

In D2983#131491, @Freagarach wrote:

Before:

After:

Some sizes of the images are not power of two.

In D1437#130020, @aeonios wrote:

I already wrote a thing that does that as part of my giant graphics upgrade. I just haven't been able to work on that and got way too many complaints about every freaking thing.

Unfortunately it's really hard to review big patches. Because the reviewing time grows exponentially with the size of patches. I'd suggest to split all big patches on as small as possible patches.

In D2812#129547, @gameboy wrote:

@Stan Then why did he ignore the trees? It's strange? Is this a flight from something?

Proper rebase.

Rebase

Fixes too late resolving.

I'd prefer to have less non-ASCII characters in the code or in the console output. Since we still support old consoles/compilers that might have problems with non-ASCII characters.

Fixes notes.

In D2945#128436, @Freagarach wrote:

I do have trouble with e.g. the theatre, but that was present before.

Too many issues.

In D2942#128020, @wraitii wrote:

In D2942#128015, @vladislavbelov wrote:

I don't see any strong reason to use std::unordered_map on the C++ side. It costs memory, some performance. The patch seems overhead, you might pass turrets through a list.

I use the map for D2814 too. That being said, you might argue that a vector of something is better for performance... Might be correct.

I don't see any strong reason to use std::unordered_map on the C++ side. It costs memory, some performance. The patch seems overhead, you might pass turrets through a list.

Indeed, there is the violation of ODR. The patch seems legit.

In D2921#126612, @Stan wrote:

Not sure why you changed the last file.

I removed the unused variable for more clear comparison of ARB and GLSL shaders.

Any change should be tested whenever possible.

In D2908#126017, @asterix wrote:

I think this is nice change and since above people also agree, I agree too but maybe more noble would be to in warning write that they need drivers with some version of graphics library and other supported shader pipelines. I cannot comment on code though.

In D2908#126009, @wraitii wrote:

It might be better to exit from the GUI, and make the GUI message "more red" to inform the player better. Not sure, your call imo.

We can't do something in GUI, because we can't render our GUI without "fixed" pipeline for users who does support only "fixed" pipeline.

I've tested the patch on Windows 10, it works for me for any map settings editing. The patch looks ok, a bit wordy but ok for atlas.

The patch looks good for me. Yes, it has duplications. But at the moment we can't make it more clear that really makes sense without a lot of code.

In rP23859#43398, @asterix wrote:

It did not close https://trac.wildfiregames.com/ticket/1109

@Clockwork-Muse, sorry for the long review and thank you for the patch.

The change has broken map settings editing.

Using intrinsic functions highly depends on a context. I'd like to see a comparison test for each method of CVector3D (without the game, one might just use tests).

In D2812#119945, @Imarok wrote:

Looks really sharp.

Thx.

@vinhig Thank you for the patch!

Shouldn't the CAS license be mentioned in LICENSE.txt?

Could you please create the patch with context? (https://trac.wildfiregames.com/wiki/SubmittingPatches#Makingsomechanges)