search

TTimo / GtkRadiant

The open source, cross platform level editor for idtech games
http://icculus.org/gtkradiant/
Other
582 stars 152 forks source link

Proper working with noradiosity 1 lights #534

Closed illwieckz closed 6 years ago

illwieckz commented 6 years ago

This is an old commit by @neumond. In fact I don't know what it is supposed to fix. By the way if someone understands both the issue and the fix and is able to tell the fix is legit, it would be a shame to forget that old commit, hence that PR.

illwieckz commented 6 years ago

So I'm looking for answers to this very simple question:

What is it?

TTimo commented 6 years ago

To which q3map2 tree was this originally checked in to? When, in what context, for what game?

TTimo commented 6 years ago

Oh I see the other pull requests. Makes more sense now.

illwieckz commented 6 years ago

It was a code sitting in an older GtkRadiant branch by @neumond providing enhancements and fixes for Unvanquished game support. So this code was targetting the GtkRadiant's master branch at this time, and the GtkRadiant default q3map2 tree (not the UrT one).

Edit: So yes, I've split his branch to make PR #533 and #535, and after the split this commit was left alone. I thought it would be a bad idea to drop it without starting a talk about it first.

illwieckz commented 6 years ago

I ping some NetRadiant guys and Unvanquished guys involved in Unvanquished mapping in case of one of them would knows something about it, or would at least understands it.

So, @TimePath, @mbasaglia, @IngarKCT, @Viech, this code was written by @neumond while he was working on adding Unvanquished support to GtkRadiant and q3map2 some years ago. This code does not seems to come from NetRadiant, and it does not looks like a fix imported from elsewhere, but it looks like something written especially with Unvanquished in mind. Have you any idea about the purpose of this commit and if this could be needed, and in which case? :no_mouth:

illwieckz commented 6 years ago

To not let it fall into oblivion:

<MateosEDK> illwieckz: isn't that code made to avoid generating radiosity for dynamic lights? <MateosEDK> since they're dynamic, they shouldn't be taken into account for static lighting on light maps <TTimo> yeah that's what that patch looked like, it'll probably be fine to merge in

Viech commented 6 years ago

Radiosity refers to q3map2 tracing light from light emitting surfaces and light entities onto the lightmaps shipped with maps. I suppose that noradiosity refers to having q3map2 ignore (and not remove, with keeligths) the light entity in question such that Dæmon can use it to cast light dynamically.

TTimo commented 6 years ago

seems safe enough, pulling it in, thanks!