Re: [Flightgear-devel] Update: Sun
Hi Melchior,
that's OK. The code has to fit to the existing codebase and so I'll tidy
it up a bit.
I just didn't look at those details. No problem.
CU Mark
Melchior FRANZ wrote:
* Mark -- Thursday 20 July 2006 20:31:
After it is in cvs I'd be happy for some feedback and suggestions for
improvement.
Here's some already, mostly formal problems. (I'm the pedant here.)
The important aspects seem to be OK.
fgGetNode(/environment));
... in a constructor. Although the existence of the /environment is
pretty much guaranteed, we still make sure that the node gets created:
fgGetNode(/environment, true));
--
consistency:
+SGPath ihalopath = path , ohalopath=path;
Now, what is it? spaces around '=' (good) or not? Space in front of
comma operator?
+i_halo_color[0] = 1- (1.1* red_scat_f);
+o_halo_color[0] = 1- (1.4* red_scat_f);
Operators sticking on numbers at several places. Either spaces around
operators (good IMHO) or not. Or is they kind of unary operators? :-}
--
indentation:
+ohalo-setCallback( SSG_CALLBACK_PREDRAW, sgSunHaloPreDraw );
+ohalo-setCallback( SSG_CALLBACK_POSTDRAW, sgSunHaloPostDraw );
+
+ sun_transform-addKid( ohalo );
+ sun_transform-addKid( ihalo );
What about aligning those correctly?
--
+ if ( env_node ){
+ env_node-setDoubleValue(atmosphere/altitude-troposphere-top,r_tropo
- r_earth);
+ env_node-setDoubleValue(atmosphere/altitude-half-to-sun, alt_half);
+ }
Umm ... what about indenting that correctly?
Yes, I know, the existing code isn't consistent either. But that's no
reason to make the situation worse.
m.
-
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.phpp=sourceforgeCID=DEVDEV
___
Flightgear-devel mailing list
Flightgear-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/flightgear-devel
-
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.phpp=sourceforgeCID=DEVDEV
___
Flightgear-devel mailing list
Flightgear-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/flightgear-devel
Re: [Flightgear-devel] Update: Sun
Hi Vassilii,
it is indeed an debugging artifact. At least the using namespace std is.
The rest was existing code. I'll see what I can change.
As for the vector math, I didn't change any of that and this is all from
the original implementation.
And I don't really want to fuzz with that.
Mark
Vassilii Khachaturov wrote:
http://www.akermann.org/fgfs/simgear-sun.tar.gz
The chunk
@@ -109,6 +109,7 @@ void SGSky::build( double h_radius_m, do
// 180 degrees = darkest midnight
bool SGSky::repaint( const SGSkyColor sc )
{
+ using namespace std;
if ( effective_visibility 1000.0 ) {
enable();
dome-repaint( sc.sky_color, sc.fog_color, sc.sun_angle,
seems a debugging artifact to me.
In oursun.cxx the constants (such as sqrt_m_log01) should probably be
declared as const. Per-coordinate vector calculations there could be
made to use the sg facilities of plib, but AFAIU it wouldn't change
much except for shorten the code a tiny bit. It's pretty readable as it is
anyhow, and fits in very nicely. Also, a patch to Thanks is due along
with the commit, to mention yourself :)
Thank you very much!!!
Vassilii
-
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.phpp=sourceforgeCID=DEVDEV
___
Flightgear-devel mailing list
Flightgear-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/flightgear-devel
-
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.phpp=sourceforgeCID=DEVDEV
___
Flightgear-devel mailing list
Flightgear-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/flightgear-devel
Re: [Flightgear-devel] Update: Sun
After doing some cleanup the patches are ready for review and
(hopefully) committing to cvs again.
The new versions are here:
http://www.akermann.org/fgfs/halos.tgz
http://www.akermann.org/fgfs/flightgear-sun-2006072201.tgz
http://www.akermann.org/fgfs/simgear-sun-2006072201.tgz
I tried to do some changes on the halo-texture but there was no
difference, so I'll leave the old one in the package for now.
The code was cleaned and polished and I must admit it wasn't very
consistent.
Since there are many people involved in this project it's necessary to
keep to certain conventions.-)
I didn't touch the vector calculations as Vassilii suggested though.
OK, still awaiting comments and suggestions,
Mark
BTW: Is it better to have one big file with all diffs concatenated or to
provide a file for every file changed?
Melchior FRANZ wrote:
* Mark -- Thursday 20 July 2006 20:31:
After it is in cvs I'd be happy for some feedback and suggestions for
improvement.
Here's some already, mostly formal problems. (I'm the pedant here.)
The important aspects seem to be OK.
fgGetNode(/environment));
... in a constructor. Although the existence of the /environment is
pretty much guaranteed, we still make sure that the node gets created:
fgGetNode(/environment, true));
--
consistency:
+SGPath ihalopath = path , ohalopath=path;
Now, what is it? spaces around '=' (good) or not? Space in front of
comma operator?
+i_halo_color[0] = 1- (1.1* red_scat_f);
+o_halo_color[0] = 1- (1.4* red_scat_f);
Operators sticking on numbers at several places. Either spaces around
operators (good IMHO) or not. Or is they kind of unary operators? :-}
--
indentation:
+ohalo-setCallback( SSG_CALLBACK_PREDRAW, sgSunHaloPreDraw );
+ohalo-setCallback( SSG_CALLBACK_POSTDRAW, sgSunHaloPostDraw );
+
+ sun_transform-addKid( ohalo );
+ sun_transform-addKid( ihalo );
What about aligning those correctly?
--
+ if ( env_node ){
+ env_node-setDoubleValue(atmosphere/altitude-troposphere-top,r_tropo
- r_earth);
+ env_node-setDoubleValue(atmosphere/altitude-half-to-sun, alt_half);
+ }
Umm ... what about indenting that correctly?
Yes, I know, the existing code isn't consistent either. But that's no
reason to make the situation worse.
m.
-
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.phpp=sourceforgeCID=DEVDEV
___
Flightgear-devel mailing list
Flightgear-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/flightgear-devel
-
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.phpp=sourceforgeCID=DEVDEV
___
Flightgear-devel mailing list
Flightgear-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/flightgear-devel
[Flightgear-devel] Update: Sun
Hi, since I have finished the integration of my suncode in the environment I'd like to set the implementation in the wild. So far everything is ready for committing it to cvs. The patches are here: http://www.akermann.org/fgfs/simgear-sun.tar.gz http://www.akermann.org/fgfs/flightgear-sun.tar.gz The changes include modifications in simgear as well as in flightgear and should be committed to both at the same time. There's also the sun-texture pack that has to be put into cvs including these textures: Textures/Sky/inner_halo.rgba Textures/Sky/outer_halo.rgba Textures/Sky/sun.rgba You can find the texture pack here: http://www.akermann.org/fgfs/halos.tgz After it is in cvs I'd be happy for some feedback and suggestions for improvement. You can change the sun's appearance by changing altitude and visibility and it also will react to humidity (which is out of the users control). TIA, Mark P.S.: Who can I send patches for simgear without penetrating the list? - Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT business topics through brief surveys -- and earn cash http://www.techsay.com/default.php?page=join.phpp=sourceforgeCID=DEVDEV ___ Flightgear-devel mailing list Flightgear-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/flightgear-devel
Re: [Flightgear-devel] Update: Sun
* Mark -- Thursday 20 July 2006 20:31:
After it is in cvs I'd be happy for some feedback and suggestions for
improvement.
Here's some already, mostly formal problems. (I'm the pedant here.)
The important aspects seem to be OK.
fgGetNode(/environment));
... in a constructor. Although the existence of the /environment is
pretty much guaranteed, we still make sure that the node gets created:
fgGetNode(/environment, true));
--
consistency:
+SGPath ihalopath = path , ohalopath=path;
Now, what is it? spaces around '=' (good) or not? Space in front of
comma operator?
+i_halo_color[0] = 1- (1.1* red_scat_f);
+o_halo_color[0] = 1- (1.4* red_scat_f);
Operators sticking on numbers at several places. Either spaces around
operators (good IMHO) or not. Or is they kind of unary operators? :-}
--
indentation:
+ohalo-setCallback( SSG_CALLBACK_PREDRAW, sgSunHaloPreDraw );
+ohalo-setCallback( SSG_CALLBACK_POSTDRAW, sgSunHaloPostDraw );
+
+ sun_transform-addKid( ohalo );
+ sun_transform-addKid( ihalo );
What about aligning those correctly?
--
+ if ( env_node ){
+ env_node-setDoubleValue(atmosphere/altitude-troposphere-top,r_tropo -
r_earth);
+ env_node-setDoubleValue(atmosphere/altitude-half-to-sun, alt_half);
+ }
Umm ... what about indenting that correctly?
Yes, I know, the existing code isn't consistent either. But that's no
reason to make the situation worse.
m.
-
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.phpp=sourceforgeCID=DEVDEV
___
Flightgear-devel mailing list
Flightgear-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/flightgear-devel
Re: [Flightgear-devel] Update: Sun
http://www.akermann.org/fgfs/simgear-sun.tar.gz
The chunk
@@ -109,6 +109,7 @@ void SGSky::build( double h_radius_m, do
// 180 degrees = darkest midnight
bool SGSky::repaint( const SGSkyColor sc )
{
+ using namespace std;
if ( effective_visibility 1000.0 ) {
enable();
dome-repaint( sc.sky_color, sc.fog_color, sc.sun_angle,
seems a debugging artifact to me.
In oursun.cxx the constants (such as sqrt_m_log01) should probably be
declared as const. Per-coordinate vector calculations there could be
made to use the sg facilities of plib, but AFAIU it wouldn't change
much except for shorten the code a tiny bit. It's pretty readable as it is
anyhow, and fits in very nicely. Also, a patch to Thanks is due along
with the commit, to mention yourself :)
Thank you very much!!!
Vassilii
-
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.phpp=sourceforgeCID=DEVDEV
___
Flightgear-devel mailing list
Flightgear-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/flightgear-devel
