On Oct 12, 2014, at 11:56 AM, Ștefan-Gabriel Mirea <mireastefangabr...@gmail.com> wrote:
> I was looking to the next issue in the BUGS file: ged_gqa() directly > calls rt_new_rti() instead of using rt_dirbuild() to create a rt_i > structure, which causes the LIBRT_BOT_MINTIE environment variable to > be ignored. Nice one... > I thought of some possible solutions to fix this, but none of them > looks elegant to me. An elegant solution eliminates the rt_bot_mintie global variable. Stashing the setting into a raytrace instance pointer (rtip) when one is created should do the trick. We could merely access the environment value when needed but unfortunately this is also exposed as a user option in at least a couple places. > A trivial fix would be to copy the two lines from rt_dirbuild() where > the rt_bot_mintie global variable is set into ged_gqa(). This is > sufficient as everything else is unnecessary: lines 53 and 54 from > librt/dir.c are equivalent to lines 80 and 81 from librt/wdb.c, that > are executed when main() in gtools/gqa.c calls ged_open(), which then > calls wdb_dbopen(). Then, a database instance is created, nearly the > same way as gedp->ged_wdbp->dbip is created using _ged_open_dbip() > (the latter is also initialized in wdb_init(), called from > wdb_dbopen()). When in doubt, rule of three [1] (i.e., allow no more than two instances of duplication). Or if you want to be elegant, the DRY principle [2] but we’re not yet very good adhering to that yet ourselves. [1] http://en.wikipedia.org/wiki/Rule_of_three_(computer_programming) [2] http://en.wikipedia.org/wiki/Don%27t_repeat_yourself > Since this is not in the spirit of code reuse and the comment above > the rt_i declaration says that such a structure must be created using > rt_dirbuild(), I think that another way would be to call rt_dirbuild() > using the database name from gedp->ged_wdbp->dbip->dbi_filename, then > forcibly associating the returned rtip with gedp->ged_wdbp->dbip. Note > that argv in ged_gqa() does not include the filename as it was removed > from the list in gtools/gqa.c and trying to change this behaviour > would lead to a huge code reorganization, because the ged_gqa() > function is also used in libtclcad/tclcad_obj.c and mged/setup.c to > create Tcl and MGED commands, which do not receive the database name > as a parameter. That’s possible but will likely be tricky to get right, be a potential race condition, and could still be a problem for future and external applications that don’t know rt_dirbuild() was the one and only true way (and it’s not, the comment is wrong/out-of-date). I wouldn’t attempt it… :) > What would be the best approach? I’d suggest either #1 or #2: #1 Move rt_bot_mintie into the rtip structure. With that working and the global eliminated, you then just have to ensure the value is set when an rtip is created/initialized (probably within rt_new_rti() which rt_dirbuild() calls). #2 Eliminate rt_bot_mintie global altogether. Make rt_bot_prep() read the environment variable on-demand (which is called during rt_dirbuild()). Any user tools that expose a bot_mintie option (e.g., nirt, rt, and archer) will have to either have that removed (user must use env var) or they’ll have to get updated to putenv/setenv instead of setting a global. Either would be acceptable and #1 is easier, but #2 is also appealing because it eliminates an unnecessary option (we should automatically be using whichever is faster or optimize TIE prep so we can eliminate the old method wholesale). Cheers! Sean ------------------------------------------------------------------------------ Comprehensive Server Monitoring with Site24x7. Monitor 10 servers for $9/Month. Get alerted through email, SMS, voice calls or mobile push notifications. Take corrective actions from your mobile device. http://p.sf.net/sfu/Zoho _______________________________________________ BRL-CAD Developer mailing list brlcad-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/brlcad-devel
