| 00:00.42 | starseeker | hah! |
| 00:00.48 | starseeker | http://brlcad.org/~starseeker/oriented_bbox_MGED.png |
| 00:30.13 | starseeker | contacted the author of this code and he agreed to dual license it with GPL2 and LGPL2.1: |
| 00:30.17 | starseeker | http://valis.cs.uiuc.edu/~sariel/research/papers/00/diameter/diam_prog.html |
| 00:32.13 | starseeker | algorithm from this paper: http://valis.cs.uiuc.edu/~sariel/papers/98/bbox.html |
| 00:33.01 | starseeker | small and self contained - wasn't hard at all to integrate and add as an option to the bb command |
| 00:41.20 | *** join/#brlcad KimK (~Kim__@wsip-184-176-200-171.ks.ks.cox.net) |
| 01:23.59 | brlcad | starseeker: what am I looking at there? |
| 01:24.43 | starseeker | the blue box is the axis aligned bounding box generated by the "standard" bb command |
| 01:25.02 | starseeker | the green box is the oriented bounding box |
| 01:25.23 | starseeker | as calculated by the algorithms from those two papers I posted |
| 01:26.05 | starseeker | the oriented box is a lot "tighter" to the actual geometry |
| 01:26.12 | brlcad | ahh! and OBB .. I was hoping that, but wasn't clear from the red lines |
| 01:26.20 | starseeker | ah sorry |
| 01:26.34 | starseeker | yeah, that's our standard "make" arbn turned into a bot, and then rotated to a funky position |
| 01:27.08 | brlcad | doesn't look like it make a proper bot |
| 01:27.48 | starseeker | that's a bad angle to judge from - it's just make arbn.s arbn followed by facetize arbn.bot arbn.s |
| 01:29.09 | starseeker | brlcad: I have it almost ready to commit, but it doesn't (yet) have the proper windows dll export/import bits and I need to clean up a couple exact floating point comparisons |
| 01:30.04 | brlcad | yeah, I believe it's the arbn .. just doesn't look like it at a glance :) |
| 01:30.44 | starseeker | heh - chose the viewing angle for contrast between the two bounding boxes - perspective probably would have helped the arbn, in retrospect... |
| 01:30.52 | brlcad | i see it's just the way it's tesselating, so many points going to the corners (instead of some midpoint balancing) |
| 01:30.58 | starseeker | nods |
| 01:31.28 | brlcad | turning it into a librt routine, yes? |
| 01:31.35 | brlcad | OOB is pretty fundamental |
| 01:31.38 | brlcad | OBB |
| 01:31.48 | starseeker | yeah - rt_bot_oriented_bbox |
| 01:32.03 | brlcad | aw |
| 01:32.05 | starseeker | obviously nmg, arbn, and a few others will be gimmies |
| 01:32.16 | starseeker | hmm? |
| 01:32.38 | starseeker | I could expose it more generally as accepting an array of points, if that's what you mean... |
| 01:32.41 | starseeker | haven't done that yet |
| 01:33.03 | brlcad | aa-bbox is a primitive functab, o-bbox is right up there with it |
| 01:33.09 | starseeker | right |
| 01:33.11 | brlcad | would be nice to generalize to all objects |
| 01:33.20 | starseeker | I did - it's a new entry in table.c |
| 01:33.21 | brlcad | even if under the hood, it's all points |
| 01:34.12 | starseeker | brlcad: I didn't commit it yet, but I posted the patch here if you want to take a look: http://brlcad.org/~starseeker/oriented_bbox.patch |
| 01:35.03 | brlcad | likes the fact that this is an O(n) algorithm, that's pretty amazing |
| 01:35.38 | starseeker | under the hood it takes a tolerance - at the moment I'm just feeding it our standard dist tolerance |
| 01:37.48 | starseeker | could undoubtely take the gdiam code as a starting point and make it use a bunch of libbu/bn routines and data types, but since it's LGPL even after the re-license I opted to maintain the separation |
| 01:38.25 | brlcad | few comments if you're interested |
| 01:38.31 | starseeker | sure, fire away |
| 01:39.07 | brlcad | their gdiam_point_t is compatible with our point_t |
| 01:39.09 | brlcad | it's identical |
| 01:39.18 | starseeker | just cast it? |
| 01:39.28 | brlcad | so you don't need the malloc in rt_bot_oriented_bbox() |
| 01:39.31 | brlcad | right |
| 01:41.08 | brlcad | the signature of the new callback also isn't consistent with the other callbacks |
| 01:41.14 | starseeker | ah, yeah - that was a memory leak anyhow (*turns red*) |
| 01:41.36 | starseeker | oh - are they all consistent? |
| 01:41.44 | brlcad | that are (mostly?? I've been meaning to do an audit) .. callback(output, input1, input2, ...) |
| 01:41.54 | starseeker | OH, gotcha |
| 01:42.51 | brlcad | not a big deal because that's not public API, but just noticed the value being returned was effectively in the middle of the list |
| 01:42.58 | starseeker | nods |
| 01:43.14 | starseeker | sorry, I thought you ment like libged where everything took the same inputs... |
| 01:44.28 | brlcad | nah |
| 01:45.57 | brlcad | believe it or not, I think that's it, looks great to me |
| 01:46.34 | starseeker | sweet! |
| 01:46.37 | brlcad | i've longed after an obb routine for a long time, some fun to be had there (particularly for editing) |
| 01:47.02 | starseeker | the author is travelling this week so it'll be a little bit before he can update his website |
| 01:47.18 | starseeker | he said to go ahead - I can copy you on the emails if you like |
| 01:52.26 | starseeker | (patch updated with changes) |
| 01:52.30 | brlcad | go ahead, we can pull it if he doesn't follow through |
| 01:52.38 | starseeker | nods |
| 01:53.09 | starseeker | NEWS worth, I take it? |
| 01:53.14 | starseeker | oh, need to update bb man page |
| 01:53.22 | brlcad | for the bb command, sure |
| 02:12.44 | Notify | 03BRL-CAD:brlcad * 56541 (brlcad/trunk/src/mged/chgview.c brlcad/trunk/src/mged/cmd.h brlcad/trunk/src/mged/setup.c): more dead code elimination: viewget, viewset, vrot_center |
| 02:22.57 | Notify | 03BRL-CAD:starseeker * 56542 (brlcad/trunk/INSTALL brlcad/trunk/configure and 6 others): Add a -o option to the bb command for generating oriented bounding boxes instead of the axis-aligned boxes (the default case). Currently only works on individual BoT objects, but can be expanded to other types in the future. Contacted author of the gdiam code (http://valis.cs.uiuc.edu/~sariel/papers/98/bbox.html) and he agreed |
| 02:22.59 | Notify | to dual-license the code under GPLv2 and LGPL 2.1, the latter of which is compatible with BRL-CAD. A slightly modified version of this code is now integrated into src/other and forms the core algorithmic basis for the bb -o option. |
| 02:24.30 | starseeker | aaand the Windows build is now broken |
| 02:26.03 | starseeker | just for added fun, I finally have a working example of http hyperlinks in the DocBook HTML man page output |
| 02:45.13 | Notify | 03BRL-CAD:brlcad * 56543 brlcad/trunk/src/proc-db/masonry.c: enable the large portions of dead code for the mortar_brick() and brick() routines, just print a loud warning to say they're untested. |
| 02:45.35 | Notify | 03BRL-CAD:brlcad * 56544 brlcad/trunk/src/proc-db/breplicator.cpp: elimiate dead code |
| 03:08.00 | Notify | 03BRL-CAD:phoenixyjll * 56545 brlcad/trunk/src/libbrep/intersect.cpp: ws |
| 03:44.24 | Notify | 03BRL-CAD:brlcad * 56546 brlcad/trunk/src/proc-db/torii.c: fix an index bug in the dynamic recursion, add a note to actually make the tool output (there was logic implemented for this somewhere...) |
| 03:47.11 | Notify | 03BRL-CAD:brlcad * 56547 brlcad/trunk/src/proc-db/molecule.c: reduce to something in-between just to avoid preprocessor dead code. factor needs to be configurable or auto-set. |
| 04:00.50 | Notify | 03BRL-CAD:brlcad * 56548 brlcad/trunk/src/rt/do.c: took me a while to get why mike added this snippet, but it's basically an example of how (via code) to reverse construct the model2view matrix from the information output to the rt log. keep it for reference, but make it code comment documentation instead of dead code. |
| 04:02.53 | Notify | 03BRL-CAD:brlcad * 56549 brlcad/trunk/src/rt/opt.c: need a way to run-time toggle between different spatial partitioning methods (including an 'off' method). |
| 04:03.50 | Notify | 03BRL-CAD:mohitdaga * 56550 brlcad/trunk/src/libicv/operations.c: Improve Bitwise operations in libicv/operations |
| 04:04.22 | zero_level | ``Erik : 56550. |
| 04:05.21 | Notify | 03BRL-CAD:brlcad * 56551 brlcad/trunk/src/rt/view.c: document why this block is retained. draws pretty status line during ray tracing. |
| 04:09.28 | Notify | 03BRL-CAD:brlcad * 56552 (brlcad/trunk/src/rttherm/pixtest.c brlcad/trunk/src/rttherm/spectrum.c): remove dead code |
| 04:10.51 | Notify | 03BRL-CAD:brlcad * 56553 brlcad/trunk/src/sig/dfft.c: eliminate dead code |
| 04:20.49 | Notify | 03BRL-CAD:brlcad * 56554 (brlcad/trunk/src/librt/primitives/brep/brep.cpp brlcad/trunk/src/librt/primitives/brep/brep_debug.cpp): normally might convert this to some #ifdev USE_* or DEBUG_* wrapping, but it's really a substantial amount of debug printing code... can revert if this is actively being used. |
| 04:22.12 | Notify | 03BRL-CAD:brlcad * 56555 brlcad/trunk/src/librt/primitives/bot/bot_wireframe.cpp: this looks interesting/significant but no indication as to why it's if 0'd, so trimming it away. easy to revert if needed. |
| 04:22.28 | Notify | 03BRL-CAD:brlcad * 56556 (brlcad/trunk/src/librt/cut.c brlcad/trunk/src/librt/primitives/arb8/arb8.c): remove dead code |
| 04:29.13 | brlcad | zero_level: please don't sugar coat the commit log messages |
| 04:29.28 | brlcad | that doesn't help anyone and just makes it harder to trace history properly |
| 04:33.19 | brlcad | that wasn't an improvement on bitwise ops, they WEREN'T bitwise. |
| 04:33.24 | brlcad | something like "fixed a bug using logical operators where bitwise operators were intended (thx for catching erik)" is far more informative and accurate |
| 04:36.30 | Notify | 03BRL-CAD:brlcad * 56557 (brlcad/trunk/src/librt/primitives/ebm/ebm.c brlcad/trunk/src/librt/primitives/ehy/ehy.c brlcad/trunk/src/librt/primitives/ell/ell.c): more dead code elimination |
| 04:59.58 | zero_level | brlcad : Well have to confess, I have to be more vary with the commit logs. Will try to improve. And ofcourse sugar caoting wasnt the intention. |
| 05:01.03 | zero_level | brlcad : about the validation issue of factor. Will defintetely, work on it. |
| 05:01.09 | zero_level | By wrapper i meant. |
| 05:02.04 | zero_level | Since the function shrink_image has a local scope. It will not be listed in the global api list. |
| 05:02.45 | zero_level | Therefore It expects validated input. |
| 05:03.03 | zero_level | which was indeed the primary though behind this. |
| 05:04.20 | zero_level | I am curretly stuck in pixhalve. |
| 05:04.52 | Notify | 03BRL-CAD:brlcad * 56558 brlcad/trunk/src/librt/primitives/nmg/nmg_ck.c: substantial implementation, but hasn't been active in a crazy long time. still keep with the relevant comments. |
| 05:04.59 | zero_level | It sort of tries to decimate the image with filter3, filter5 |
| 05:06.04 | zero_level | Once this gets completed. I plan to write a wraper for the scale, decimate, interpolate smth like icv_scale(..) |
| 05:06.33 | brlcad | zero_level: it's more about not being incorrect in the commit logs, owning up to any bugs should be accepted, they will happen |
| 05:06.34 | zero_level | a global api. which will take care of the all resizing issues. |
| 05:07.26 | brlcad | the problem is that purely general functions like that often and eventually get repurposed or refactored later |
| 05:07.36 | brlcad | maybe years later long after you are no longer involved |
| 05:07.58 | brlcad | and a function that expected validated input is moved to some place else without getting fully reviewed, and a crash results |
| 05:08.29 | brlcad | there's no reason even for static scope functions that they should not test their assumptions |
| 05:08.44 | zero_level | ok. |
| 05:09.20 | brlcad | you can make the test insignificant performance-wise by marking them with UNLIKELY() and LIKELY() |
| 05:09.45 | brlcad | if (UNLIKELY(factor == 0)) |
| 05:09.59 | brlcad | <PROTECTED> |
| 05:10.08 | brlcad | even something that extreme would work |
| 05:10.26 | brlcad | though a graceful return is usually preferred if you can avoid it |
| 05:10.44 | brlcad | the intent is exactly to prevent a crash, so the test and action can be exceptional |
| 05:10.54 | brlcad | division by zero in that function's case |
| 05:10.56 | zero_level | alright. If maintainability is the issue. I then will have to add validation in a lot of code. |
| 05:11.41 | zero_level | will then have to exploit the use UNLIKELY. :-) |
| 05:12.31 | brlcad | you are creating a new library .. if it's not added now, it probably won't get added later |
| 05:12.43 | zero_level | brlcad : ok. |
| 05:13.00 | brlcad | better to have fewer functions, heck only ONE function if it took you all summer to get it perfect in my opinion |
| 05:13.14 | brlcad | the other issue I'm hoping you get to is actually using and validating libicv |
| 05:13.35 | brlcad | as it stands, you're just increasing the size of BRL-CAD with little gain |
| 05:13.46 | zero_level | brlcad : :-) |
| 05:13.49 | brlcad | making all of those tools use your functions needs to be more proactively integrated |
| 05:14.05 | brlcad | and is part of coding complete |
| 05:15.15 | zero_level | yes. Thats an issue I wld like to discuss with all of you. |
| 05:15.37 | zero_level | do you think we should merge utilities. |
| 05:16.02 | brlcad | i'd like to do a code review, perhaps later this week to see what we might change going forward |
| 05:16.09 | brlcad | merge utilities? |
| 05:16.13 | zero_level | like bwshrink and pixshrink or let them be alone |
| 05:17.03 | brlcad | so yeah, eventually yes but probably not before gsoc is over |
| 05:17.12 | brlcad | I'd rather see refactoring dictate that |
| 05:17.13 | zero_level | b) Most of the utilities currently use a very efficient manner. By not loading the complete image. |
| 05:17.34 | brlcad | that you integrate icv into those two routines first, then it become obvious that they are (now) really the same routine and can be merged |
| 05:18.04 | zero_level | although current structure of using icv is very easy. |
| 05:18.17 | brlcad | efficient in terms that are not necessarily relevant (and perhaps even harmful) |
| 05:18.18 | zero_level | icv_load() operations icv_save(). |
| 05:18.29 | zero_level | then its fine. |
| 05:18.44 | brlcad | we're not going to just guess |
| 05:18.53 | brlcad | this all has to be tested, profiled, and compared |
| 05:19.07 | zero_level | ok. |
| 05:20.38 | brlcad | at this point, I'm inclined to say pause groups 9-11 for now so that you can start on integration and testing earlier |
| 05:21.36 | brlcad | your plan called for unit testing and code coverage concurrent with all this code migration, which you haven't even started on and you're up to group 8 |
| 05:21.51 | brlcad | needed to integrate and test after group 1 |
| 05:22.17 | brlcad | which the rt* tools kind of was, but that was a literal disaster that we're still recovering from |
| 05:23.08 | brlcad | you're doing great work, but I think you're getting too far ahead of the plan without testing adequately |
| 05:28.23 | zero_level | Well I unit tested in my local trunk. |
| 05:28.38 | zero_level | by writting main.c |
| 05:28.56 | zero_level | and as I said earlier tested them with matlab. |
| 05:29.21 | zero_level | I didnt find ways to automatically test them (as in bu) |
| 05:31.20 | Notify | 03BRL-CAD:brlcad * 56559 brlcad/trunk/src/librt/primitives/nmg/nmg_ck.c: need the return statement, pull ifndef up |
| 05:33.46 | Notify | 03BRL-CAD:brlcad * 56560 brlcad/trunk/src/librt/primitives/nmg/nmg_fcut.c: remove three dead and long-unused functions that have been under if 0 wrappage: nmg_insert_vu_if_on_edge(), nmg_face_combineX(), and nmg_face_next_vu_interval(). they even seem interesting/complete, but are not integrated, easily tested, or fully understood so remove the burden. nmg has enough complexity without retaining some for reference. |
| 05:33.51 | brlcad | zero_level: you misunderstand perhaps |
| 05:33.59 | brlcad | there's testing a function and there's testing functionality |
| 05:34.30 | brlcad | you're pulling this logic from src/util utilities .. so you have potential integrating testing material right there |
| 05:36.11 | zero_level | brlcad : there are some issues doing this with the util way because of the use of double data. |
| 05:36.15 | zero_level | for eg. |
| 05:36.16 | brlcad | you run a utility, capture a result, and create a test that verifies that result. then instead of copying the code, you MOVE it, thus removing it from where you copied it from and making the old utility use your new function. then you make sure that the tool works as it did before (by running the integration test you created) |
| 05:36.46 | brlcad | so? :) |
| 05:36.59 | brlcad | issues != impossibilities |
| 05:37.05 | brlcad | they are not secondary to the task |
| 05:37.13 | brlcad | they are part of it |
| 05:37.15 | zero_level | bwmod(..) |
| 05:37.21 | brlcad | if those issues cannot be sorted out easily, libicv will fail and die... |
| 05:37.49 | zero_level | can I complete about bwmod. :-) |
| 05:38.09 | brlcad | it depends |
| 05:38.16 | zero_level | bwmod creates a look aside buffer to imlement |
| 05:38.53 | zero_level | since the utilities use char data the buffer length is 256. |
| 05:39.05 | zero_level | But we cannot use buffer in double data. |
| 05:39.17 | zero_level | bwmod has been translated to operations. |
| 05:39.52 | zero_level | icv_add_val, icv_mul_val... |
| 05:41.14 | zero_level | but other than these all other are transferable. |
| 05:42.05 | brlcad | so we need to clarify some language you're using |
| 05:42.12 | brlcad | at least I need you to clarify some things |
| 05:42.44 | brlcad | "But we cannot use buffer in double data." does not make sense, of course I can have a buffer of double data -- what did you mean? |
| 05:42.58 | zero_level | alright. |
| 05:43.33 | zero_level | bwmod is a unique utility which can have a lot of constant value operations |
| 05:44.02 | brlcad | sure, a lot of per-pixel math operations |
| 05:44.10 | zero_level | -a5 -d20 -m0.5 -d10 -a12 |
| 05:44.45 | brlcad | that is what it is .. I understand that |
| 05:44.57 | zero_level | makes my job easy |
| 05:44.58 | brlcad | I don't understand your comment about double data buffers |
| 05:45.11 | Notify | 03BRL-CAD:starseeker * 56561 brlcad/trunk/NEWS: Added -o option to bb bounding box command - provides an oriented bounding box rather than the default axis-aligned bounding box. |
| 05:45.27 | zero_level | so this is implemented using a buffer for all the possible values of pixels (0-255) |
| 05:45.59 | brlcad | sure |
| 05:46.23 | brlcad | which in icv terms should be a 1-channel image |
| 05:46.26 | zero_level | so val[i] will ave ((((i+5)/20)*0.5)/10)+12 |
| 05:46.33 | zero_level | *have |
| 05:46.43 | zero_level | now since in icv we use double data. |
| 05:47.07 | zero_level | there are infinite possibilies 0.0-1.0 |
| 05:47.08 | brlcad | yes, sure |
| 05:47.16 | brlcad | well, not infinite, but yes a lot |
| 05:47.31 | brlcad | and? |
| 05:47.32 | zero_level | well ofcourse in computer terms not infinite. |
| 05:47.42 | zero_level | so i cannot have a buffer. |
| 05:47.48 | brlcad | untrue |
| 05:47.50 | zero_level | Indeed i have to do this as following |
| 05:48.30 | zero_level | icv_add_val(bif,5); icv_divide_val(bif,20); icv_multiplyt_val(bif,0.5); |
| 05:48.34 | zero_level | and so on. |
| 05:48.51 | zero_level | with setting the ICV_OPERATIONS_FLAG. |
| 05:49.04 | zero_level | now issues. |
| 05:49.12 | brlcad | slow down |
| 05:49.20 | brlcad | remember this is a discussion, not an explanation |
| 05:49.26 | zero_level | sure :-) |
| 05:49.54 | brlcad | what you're saying is not possible is certainly possible, though it could very well just be a terminology problem |
| 05:50.05 | zero_level | ok. |
| 05:50.06 | brlcad | libicv IS your buffer |
| 05:50.11 | zero_level | ? |
| 05:50.11 | brlcad | it's an opaque container |
| 05:50.24 | brlcad | you put data in, you tell it to do things to the data, you get data out |
| 05:50.25 | zero_level | u mean icv_image_t |
| 05:50.31 | zero_level | ofocurse. |
| 05:50.31 | brlcad | yes |
| 05:51.06 | brlcad | the fact that bwmod has a bunch of per-pixel operations doesn't change anything |
| 05:51.16 | brlcad | if it does, then there is some understanding mismatch |
| 05:51.21 | zero_level | but in bwmod it will bw very easy fast because it does as . im[j] = val[im[j]] |
| 05:51.27 | brlcad | so? |
| 05:51.33 | brlcad | who said anything about easy? |
| 05:51.44 | brlcad | and speed is not a concern in the least |
| 05:51.51 | brlcad | not unless you profile and show it matters |
| 05:52.21 | zero_level | ok. |
| 05:52.26 | brlcad | or at least until some tool that used to run instantly now takes many minutes... |
| 05:52.41 | brlcad | but that's not likely and will be obvious if it happens |
| 05:53.11 | brlcad | so in bwmod's case, the issue is the operations being performed on the data |
| 05:53.39 | zero_level | so u mean what earlier took 10 ms if it takes 15 ms. there is no issue. :-) |
| 05:53.47 | brlcad | before it was a direct im[j] = im[j] + val1 - val2 etc |
| 05:54.23 | brlcad | but now, you not longer are dealing with 'im', you've got 'icv(im)' |
| 05:54.24 | zero_level | did u miss look aside buffer ? :-) |
| 05:54.36 | brlcad | and instead of val1, you've got icv(val1) etc |
| 05:54.52 | brlcad | yes, a 5 ms diff would be pretty much irrelevant |
| 05:55.03 | zero_level | it basically creates a table for all the possible values. |
| 05:55.06 | brlcad | no, the look aside buffer is also irrelevant |
| 05:55.11 | zero_level | and stores them in val. |
| 05:55.27 | zero_level | (an eg here) |
| 05:55.36 | brlcad | so again, the question is how to get ICV to do that work as much possible |
| 05:55.50 | zero_level | thats not a question. |
| 05:56.08 | zero_level | The question was does this performance matter. |
| 05:56.17 | zero_level | and as u said we cannot be guessing |
| 05:56.20 | brlcad | sure it is a question: "how can you get ICV to do that work as much as possible?" |
| 05:56.22 | zero_level | we need to profile |
| 05:56.36 | brlcad | eh? |
| 05:56.45 | zero_level | profiling is the way. |
| 05:57.29 | zero_level | brlcad : the implementation was as follows. As I told earlier |
| 05:57.30 | zero_level | 01:48 < zero_level> icv_add_val(bif,5); icv_divide_val(bif,20); icv_multiplyt_val(bif,0.5); |
| 05:57.33 | zero_level | 01:48 < zero_level> and so on. |
| 05:57.36 | zero_level | 01:48 < zero_level> with setting the ICV_OPERATIONS_FLAG. |
| 05:57.38 | zero_level | 01:49 < zero_level> now issues. |
| 05:58.06 | brlcad | profiling is always welcome but I don't see what for (yet) as there's no indication of a performance concern |
| 05:58.16 | zero_level | alright. |
| 05:58.32 | brlcad | I'd say you can ASSUME there is not a performance concern until we observe one |
| 05:58.33 | zero_level | which indeed answers what i started with. :-) |
| 05:59.01 | brlcad | if you're going to make ANY design decision BASED on a performance concern, you need to profile |
| 05:59.10 | brlcad | that has been stated from the beginning |
| 05:59.41 | zero_level | do u think grpof suffices here. |
| 05:59.45 | brlcad | back to the implementation you outlined, that does indeed sound a little problematic, at least not a faithful conversion of what bwmod's doing |
| 05:59.59 | zero_level | how ? |
| 06:00.09 | zero_level | i wld welcome a better suggestion. |
| 06:01.27 | brlcad | it's back to the notion I mentioned earlier: you put data in, you tell it to do things to the data, you get data out |
| 06:01.45 | brlcad | so if the old code was doing im[j] = val[im[j]] |
| 06:02.10 | brlcad | that translates roughly into ICV needing to do several things and bwmod needing to do several things |
| 06:02.48 | brlcad | two icv_image_t's: let's call them icv_im and icv_val |
| 06:03.01 | zero_level | well.. |
| 06:03.04 | brlcad | a way to set one icv_image to another |
| 06:03.25 | zero_level | can i hold u for a sec |
| 06:03.29 | brlcad | a way to access and set/modify elements (which you partially have) |
| 06:03.37 | brlcad | go ahead |
| 06:03.51 | zero_level | val is an array of length 0-255. |
| 06:03.54 | zero_level | so 256. |
| 06:03.58 | brlcad | not in icv terms |
| 06:04.02 | zero_level | and im is buffer. |
| 06:04.07 | zero_level | of scalen length |
| 06:04.35 | zero_level | so then i dont understand your point of using two images. |
| 06:04.37 | brlcad | val sounds like a 1-dimensional buffer to me |
| 06:05.17 | zero_level | a) i would loop along all the pixels of image (bif) in first go and add a value to it. |
| 06:05.59 | zero_level | brlcad : ofcourse thats what i meant (a buffer of size 0 to 255 i.e 256) |
| 06:06.48 | brlcad | do you really mean the size is variant 0 to 255 or the range of values is 0 to 255? |
| 06:06.55 | zero_level | no.! |
| 06:07.03 | brlcad | that wasn't a yes no question |
| 06:07.09 | zero_level | opens bwmod to give exact terminologies. |
| 06:07.11 | brlcad | A or B |
| 06:07.48 | zero_level | brlcad : do u have acess to the src code ? |
| 06:07.54 | zero_level | i am sure u do. :-) |
| 06:08.26 | zero_level | (I mean i suppose u are not on a cell phone) |
| 06:08.29 | brlcad | a "size" is generally the thing you pass to malloc, the type (e.g., unsigned char) is the range of possible values |
| 06:08.37 | zero_level | yes. |
| 06:08.55 | zero_level | so look at the function mk_trans_tbl in bwmod.c |
| 06:09.18 | zero_level | what it does is creates a translation look aside buffer |
| 06:09.24 | zero_level | of size 256. |
| 06:09.32 | *** join/#brlcad caen23 (~caen23@92.83.187.81) |
| 06:09.40 | zero_level | that is a 1-Dim array with 256 values. |
| 06:10.31 | zero_level | which in the case of uchar data, is the range of values. |
| 06:10.46 | brlcad | actually, it's line 68 that creates the array and mk_trans_tbl() fills it in |
| 06:11.03 | zero_level | ye. |
| 06:11.04 | brlcad | it's important to be precise |
| 06:11.34 | brlcad | especially since there is apparently some difficulty simply discussing options here |
| 06:11.55 | brlcad | so my point still stands |
| 06:12.01 | zero_level | of. So I guess now we are on the same platform as to how bwmod does things. |
| 06:12.17 | zero_level | go on. |
| 06:12.18 | brlcad | mapbuf could be an icv image of size 1x256 with values set into it and read |
| 06:12.23 | brlcad | or it could stay in bwmod |
| 06:12.29 | brlcad | doesn't really matter |
| 06:12.39 | brlcad | the point is the getting and setting of values in icv's domain |
| 06:12.54 | zero_level | well brlcad : doing that |
| 06:13.45 | zero_level | does some math to make brlcad understand. |
| 06:14.28 | zero_level | say a pixel with value 0.004 and 0.006 (in the new icv terms_ |
| 06:14.28 | brlcad | do you at least realize that there are probably a dozen perfectly valid ways this can be translated to libicv? |
| 06:14.50 | brlcad | okay, continue |
| 06:14.58 | zero_level | doing what u suggest will make them both look same way. |
| 06:15.00 | brlcad | how does one pixel have two values? |
| 06:15.12 | zero_level | well two pixels |
| 06:15.17 | brlcad | be precise |
| 06:15.18 | zero_level | so i begin again |
| 06:15.41 | starseeker | src/proc-db/masonry.c:945:9: error: variable ‘vert_bricks’ set but not used |
| 06:15.42 | zero_level | two pixels a and b with value 0.004 and 0.006 |
| 06:16.18 | zero_level | now lets suppose we create an image of size 1,256 in icv terms |
| 06:16.42 | brlcad | starseeker: fixed |
| 06:16.47 | starseeker | thanks |
| 06:16.57 | Notify | 03BRL-CAD:brlcad * 56562 brlcad/trunk/src/proc-db/masonry.c: vert_bricks unused, warning didn't issue locally but clearly there. |
| 06:16.59 | zero_level | 0.004 and 0.006 will both correspond to same data level |
| 06:17.24 | brlcad | sure |
| 06:17.52 | zero_level | thus the whole point of using double(to get high resolution) will be dismantled. |
| 06:18.12 | brlcad | so far, probably bw values 1 and 2 respectively depending on rounding behavior |
| 06:18.49 | brlcad | how did that just dismantle anything? |
| 06:18.53 | brlcad | you didn't say anything |
| 06:18.54 | zero_level | how does bot work |
| 06:19.00 | zero_level | bot : ~1/255 |
| 06:19.30 | Notify | 03BRL-CAD:starseeker * 56563 brlcad/trunk/src/librt/primitives/bot/bot_oriented_bbox.cpp: Ah, right - don't need xyz vars any more. |
| 06:20.29 | zero_level | brlcad : eh ? |
| 06:20.36 | brlcad | you're not making any sense |
| 06:21.00 | brlcad | two pixels: 0.004 and 0.006 ... and so what? |
| 06:21.23 | zero_level | 1/255 = 0.00392156 |
| 06:21.40 | zero_level | 2/255 = 0.0078125 |
| 06:22.15 | zero_level | so 0.004 and 0.006 will correspond to same data level if i use your suggestion of 1X255 image. |
| 06:22.28 | zero_level | c/1X2/1x256 |
| 06:22.35 | zero_level | c/1X255/1x256 |
| 06:22.45 | zero_level | is experiencing lots of lag. |
| 06:23.10 | brlcad | "will correspond to same data level" ... and so what? |
| 06:23.11 | zero_level | thus hampering the resolution of the image. |
| 06:24.07 | brlcad | you're still not making any sense or your making some conclusion that is completely wrong or I'm entirely not seeing whatever it is you're trying to say... |
| 06:24.49 | brlcad | first the data is coming from somewhere, so more than likely, they'd be pixels 0.00392156 and 0.0078125, not 0.004 and 0.006, if they came from bw data to begin with |
| 06:24.50 | zero_level | well lets suppose i want to output the resultant operations in highdefinition png image. |
| 06:25.06 | Notify | 03BRL-CAD:starseeker * 56564 brlcad/trunk/include/raytrace.h: Commit proposed new librt database search API function declaration and design notes - not implemented as yet, needs more thought/discussion |
| 06:25.06 | zero_level | pls hold on for a sec |
| 06:25.24 | brlcad | but even if we assume that they DID start out as 0.004 and 0.006, theose values will correspond to either a bw value of 1 or 2 (depending on rounding) |
| 06:25.42 | zero_level | not always brlcad! |
| 06:25.57 | zero_level | because in high definition png images. |
| 06:26.04 | zero_level | say 16 bits |
| 06:26.40 | zero_level | it will correspond to 0.004*65536 -1 |
| 06:26.58 | zero_level | and 0.006*65536 - 1 |
| 06:27.23 | brlcad | sure, it'll be a difference if someone asks for 16-bit png output .. but how is that at all relevant? |
| 06:27.55 | brlcad | that's why I said .. even if we assume that they DID start out as 0.004 and 0.006 ... what does that matter? |
| 06:28.10 | brlcad | if the user requests low-resolution bw output, YES the data will get quantized |
| 06:28.12 | brlcad | the user requested it! |
| 06:28.19 | zero_level | well brlcad : It all depends on what we want. |
| 06:28.23 | brlcad | if they ask for high-resolution output, they'll get that too |
| 06:28.58 | brlcad | uhm, we're performing the operation that the user requested, nothing more or less |
| 06:29.06 | zero_level | After our discussion on image container long back. I got a gut feeling that |
| 06:29.22 | zero_level | a) resolution must be given some priority. |
| 06:29.46 | brlcad | which was the point for double -- it makes all processing occur at the highest resolution readily available |
| 06:30.02 | zero_level | b) we are headed to a time in future. where high res images will be used. (like the openexr format_ |
| 06:30.17 | brlcad | again, the point for double |
| 06:30.33 | zero_level | thus making it a fixed size array doesnt help |
| 06:30.42 | zero_level | we will have to do inplace calculation. |
| 06:30.54 | brlcad | "it" being bwmod's 256 element array? |
| 06:31.21 | zero_level | well. logically bwmod will still be correct. |
| 06:31.33 | brlcad | no, tell me what you meant when you said "it" |
| 06:31.43 | brlcad | you said 'it', don't make me guess |
| 06:31.46 | zero_level | heheh :-_ |
| 06:32.01 | zero_level | well its the 1X256 image u suggested me to use. |
| 06:32.54 | zero_level | see sean, i can do it the way suggested. This will hardly take any time. |
| 06:33.22 | zero_level | but i would not like to. Because it will hamper resolution. |
| 06:33.30 | brlcad | I stated multiple times that there are multiple directions that can be taken, all valid |
| 06:33.34 | brlcad | they could be separate icv images |
| 06:33.49 | zero_level | I want icv operations not to hamper the resolutions. |
| 06:33.53 | brlcad | it could be one icv image and quantized data being still held in bwmod |
| 06:34.17 | brlcad | if you think it will hamper resolution, then I think you are completely misunderstanding something |
| 06:34.42 | zero_level | well. not in the case of bwmod. (since it uses 8bit images) |
| 06:35.05 | zero_level | but ofcoruse in the future course when we use high res images. |
| 06:36.17 | brlcad | so the issue in this specific instance is bwmod |
| 06:36.29 | zero_level | And some body does and operations only to find his/her image is equivalent to uchar image with alased double data. (like 0.00392156 fr "1") |
| 06:36.36 | brlcad | and it's current implementation uses a 256 element buffer and operations index into that buffer |
| 06:36.59 | brlcad | so your first and foremost obligation is to do that, and demonstrate it works |
| 06:37.28 | brlcad | show that works perfectly, then extend to n-channel images or 128-bit images or whatever else next |
| 06:37.31 | zero_level | well . It would still. And carrying the future course in action. |
| 06:38.04 | zero_level | 01:48 < zero_level> icv_add_val(bif,5); icv_divide_val(bif,20); icv_multiplyt_val(bif,0.5); |
| 06:38.05 | brlcad | for a 16-bit image, the entire algorithm of bwmod would have to change since you couldn't have a look-aside buffer that large to directly index into |
| 06:38.32 | brlcad | you keep saying that and I'm not sure what you're hoping it implies |
| 06:38.51 | brlcad | icv_*_val() as you state it there looks problematic |
| 06:39.05 | brlcad | what is the range of that 5/20/0.5 |
| 06:39.39 | zero_level | well it adds freedom to the api caller. |
| 06:39.53 | brlcad | if the data is being stored 0.0 to 1.0 then all inputs need to be translated to "icv units" |
| 06:39.54 | zero_level | to translate the bwmod functions. It will be whate |
| 06:40.01 | zero_level | hold on pls |
| 06:40.09 | zero_level | x/255 |
| 06:40.25 | brlcad | exactly my point |
| 06:40.26 | zero_level | x is the value given by the utility caller |
| 06:40.27 | brlcad | that's bad |
| 06:40.36 | brlcad | why would it assume 8-bit data? |
| 06:40.51 | brlcad | you can't have an api that is sometimes in one range and sometimes in another |
| 06:40.59 | brlcad | the API needs to define what it's using |
| 06:40.59 | zero_level | because bwmod does it in correct for, |
| 06:41.16 | brlcad | this is a libicv function, so that justification doesn't hold |
| 06:41.18 | zero_level | say if brl-cad in future have a utility |
| 06:41.23 | zero_level | for operations |
| 06:41.50 | zero_level | which handles all the operations. |
| 06:42.22 | brlcad | and I feed it my exr image and it messes up my data |
| 06:42.29 | zero_level | no. |
| 06:42.51 | *** join/#brlcad Ch3ck_ (~Ch3ck@195.24.220.16) |
| 06:43.04 | brlcad | do any other icv functions take an 8-bit 0-to-255 value? |
| 06:43.13 | zero_level | no. |
| 06:43.19 | zero_level | just writeline |
| 06:43.20 | brlcad | that alone should be a red flag to you |
| 06:43.44 | zero_level | writeline can handle both double data and uchar data. |
| 06:44.49 | brlcad | there should be functions for adding/extracting data in various formats |
| 06:45.10 | brlcad | but you can't have processing functions handling everything possible, they need to work with icv's units |
| 06:45.18 | brlcad | which means you need translation routines, generalized |
| 06:45.33 | brlcad | similar to what you did with a few of your other functions |
| 06:45.42 | brlcad | but still more generalized |
| 06:45.54 | *** join/#brlcad Izak_ (~Izak@66-118-151-70.static.sagonet.net) |
| 06:46.33 | brlcad | a function that converts 8-bit 0-255 1-channel data to double 0-1.0 ranged values, for example |
| 06:46.36 | brlcad | and back |
| 06:47.28 | brlcad | so it becomes something like icv_add_val(bif, icv_cnv_8bit(20)) or similar |
| 06:48.27 | *** join/#brlcad Izak__ (~Izak@195.24.220.16) |
| 06:54.47 | Notify | 03BRL-CAD:brlcad * 56565 brlcad/trunk/src/librt/primitives/nmg/nmg_inter.c: three more dead code functions from nmg, though two are relatively recent and may need revisiting at some point later. |
| 07:01.47 | Notify | 03BRL-CAD:brlcad * 56566 (brlcad/trunk/src/librt/primitives/nmg/nmg_misc.c brlcad/trunk/src/librt/primitives/nmg/nmg_mod.c brlcad/trunk/src/librt/primitives/nmg/nmg_tri.c): another 1057 lines gone, remove libnmg dead code |
| 07:06.49 | Notify | 03BRL-CAD:brlcad * 56567 brlcad/trunk/src/librt/primitives/hf/hf.c: remove if 1 wrapping |
| 07:07.53 | Notify | 03BRL-CAD:brlcad * 56568 brlcad/trunk/src/librt/primitives/pipe/pipe.c: no longer too much for mged to display |
| 07:09.06 | Notify | 03BRL-CAD:brlcad * 56569 brlcad/trunk/src/librt/primitives/tgc/tgc.c: re-enable tgc reporting of non-paired grazing hits, but only for the first 100 cases. intentionally making no attempt to be thread-safe in the book-keeping here because it doesn't matter. just need some upper limit. |
| 07:09.20 | Notify | 03BRL-CAD:brlcad * 56570 brlcad/trunk/src/librt/primitives/tgc/tgc.c: save the file first, endif too |
| 07:10.49 | Notify | 03BRL-CAD:brlcad * 56571 brlcad/trunk/src/librt/primitives/tgc/tgc.c: getting late, extra curlie |
| 07:11.09 | brlcad | starseeker: I'm leaving the numerous undocumented #if 0's in src/librt/test_bot2nurbs.cpp up to you |
| 07:12.10 | brlcad | unless you tell me I can kill them, they beg for some commentary / reasoning to keep that code |
| 07:12.29 | brlcad | src/librt/test_root3-subd.cpp too |
| 07:16.50 | Notify | 03BRL-CAD:brlcad * 56572 brlcad/trunk/NEWS: Added -o option to bb bounding box command - provides an oriented bounding box rather than the default axis-aligned bounding box. (comment was spot on, actual line was not past tense consistent) |
| 08:48.11 | zero_level | brlcad : thanks for the suggestion. |
| 08:48.35 | zero_level | icv_cnv_8bit(..) looks fine to me. |
| 11:56.51 | starseeker | brlcad: fair enough |
| 11:58.21 | starseeker | Ch3ck_: make small patches that incrementally work towards your goal |
| 11:58.35 | Ch3ck_ | ok |
| 11:58.55 | Ch3ck_ | but the patch i've already made for the pull is it ok? |
| 11:59.00 | starseeker | Ch3ck_: I'll try to review more of your patches today - I've got several irons in the fire right now |
| 11:59.10 | Ch3ck_ | hm |
| 11:59.18 | starseeker | Ch3ck_: if you mean your large existing ones, I'd try to break them up |
| 11:59.28 | Ch3ck_ | ok |
| 11:59.38 | Ch3ck_ | working on pulling non leaf objects |
| 11:59.42 | Ch3ck_ | as Sean had advice. |
| 11:59.43 | Ch3ck_ | d |
| 11:59.53 | *** join/#brlcad ejno (~ejno@unaffiliated/kazaik) |
| 11:59.55 | starseeker | if I recall correctly brlcad said it should be possible to do so, so that will be something you want to demonstrate you can do |
| 12:00.09 | Ch3ck_ | thats what i'm working on today. |
| 12:00.17 | starseeker | good |
| 12:03.09 | Ch3ck_ | i'll continue work on the pull and just increment the current patch |
| 12:03.17 | Ch3ck_ | with recent changes. |
| 12:18.42 | ``Erik | Izak__: I'm not seeing the 'barebones' patch in the sourceforge tracker |
| 12:19.23 | Izak__ | ``Erik: Let me upload it |
| 12:27.53 | Izak__ | ``Erik: I have uploaded it to ticket 228 |
| 12:37.02 | Ch3ck_ | there is this thing i don't understand with rt_db_get_internal(). The 4th argument which is a matrix given this routine. Is it the matrix transformation of the database object in external format or its the matrix transformation to be written to the internal database object. |
| 12:37.40 | Ch3ck_ | I think it is the matrix of the internal database object in external format but i just need to clarify. |
| 12:37.51 | Ch3ck_ | or confirm |
| 12:47.03 | ``Erik | Izak__: patch has trailing whitespace and table.c fails to apply cleanly on r56572 |
| 12:49.36 | *** join/#brlcad mpictor_ (~mpictor_@2600:1015:b122:7d10:0:48:5dba:4a01) |
| 12:51.11 | ``Erik | also; import4/export4 should be left NULL and not used at all |
| 12:51.27 | Notify | 03BRL-CAD:phoenixyjll * 56573 (brlcad/trunk/include/brep.h brlcad/trunk/src/libbrep/CMakeLists.txt brlcad/trunk/src/librt/primitives/brep/brep.cpp): Start to work on evaluating NURBS booleans. Move the code working with NURBS booleans in librt to libbrep. |
| 12:54.17 | ``Erik | starseeker: http://www.caranddriver.com/reviews/volkswagen-xl1-concept-first-drive-review |
| 13:04.35 | Notify | 03BRL-CAD:phoenixyjll * 56574 brlcad/trunk/src/libbrep/boolean.cpp: ws. |
| 13:05.33 | Notify | 03BRL-CAD:phoenixyjll * 56575 brlcad/trunk/src/librt/primitives/bot/bot_oriented_bbox.cpp: Eliminate compiler warnings. |
| 13:21.43 | Izak__ | ``Erik: Please can you paste the build errors you got because I have rebuilt and it succeeds |
| 13:37.13 | ``Erik | no build errors, the patch failed to apply at all |
| 13:37.32 | *** join/#brlcad kesha (~kesha@14.139.122.114) |
| 13:40.16 | Izak__ | ``Erik: I am working on the trailing white space now |
| 13:41.03 | ``Erik | make sure your tree is fully up to date, using "svn update" |
| 13:41.44 | Notify | 03BRL-CAD:phoenixyjll * 56576 (brlcad/trunk/src/libged/bb.c brlcad/trunk/src/libged/comb.c): The C compiler in MSVC seems to require all declarations appear first. |
| 13:46.32 | *** join/#brlcad kesha (~kesha@14.139.122.114) |
| 13:53.47 | Notify | 03BRL-CAD:phoenixyjll * 56577 brlcad/trunk/src/libbrep/boolean.cpp: curve_st should be the curves in surfaceB's domain. |
| 13:56.18 | Notify | 03BRL-CAD:mohitdaga * 56578 (brlcad/trunk/include/icv.h brlcad/trunk/src/libicv/encoding.c brlcad/trunk/src/libicv/fileformat.c): Add ICV_CONV_8BIT macro for encoding conversions. This macro converts the 8bit pixel values to icv data type(double). Later we can have similar macros for higher resolution of images. For eg ICV_CONV_16BIT. |
| 13:57.27 | Notify | 03BRL-CAD Wiki:Phoenix * 5923 /wiki/User:Phoenix/GSoc2013/Reports: /* Week 7 */ |
| 13:57.45 | Notify | 03BRL-CAD Wiki:Phoenix * 5924 /wiki/User:Phoenix/GSoc2013/Reports: /* Week 8 */ |
| 13:57.58 | Notify | 03BRL-CAD Wiki:Phoenix * 5925 /wiki/User:Phoenix/GSoc2013/Reports: /* Mid-term summary */ |
| 14:02.02 | zero_level | clear |
| 14:03.23 | *** join/#brlcad kesha (~kesha@14.139.122.114) |
| 14:06.32 | *** join/#brlcad kesha_ (~kesha@14.139.122.114) |
| 14:07.55 | Notify | 03BRL-CAD:carlmoore * 56579 (brlcad/trunk/include/icv.h brlcad/trunk/include/raytrace.h and 2 others): fix spelling, and make about 2 other minor changes |
| 14:18.04 | *** join/#brlcad kesha_ (~kesha@14.139.122.114) |
| 14:34.18 | brlcad | zero_level: so the detail there is that libicv won't normally know when it needs to run that 8bit conversion, but the calling code does |
| 14:34.42 | brlcad | so tools like bwmod would convert their user-provided values into the normalized 0-1 domain with ICV API |
| 14:37.22 | brlcad | Izak__: we shouldn't still have to be reminding about white space/style and that code compile cleanly against trunk... |
| 14:38.43 | starseeker | Izak__: I changed the table.c stuff just this weekend to add a method for oriented bounding boxes |
| 14:38.54 | starseeker | so the heart primitive should follow |
| 14:39.51 | Izak__ | satrseeker: Are you referring to the rt_hrt_bbox() function ? |
| 14:40.05 | starseeker | no, the definition for heart in table.c |
| 14:40.37 | starseeker | you need another NULL in the list of heart functions, since the size of the function array has changed |
| 14:40.44 | starseeker | easy to do |
| 14:44.31 | starseeker | Ch3ck_: did you see the change from brlcad to the bn_poly test? He removed the copyright information generated by octave and moved the comment within the source file |
| 14:45.31 | starseeker | Ch3ck_: (commit r56506) please make sure your other patches are updated accordingly, if need be |
| 14:46.04 | Ch3ck_ | ok |
| 14:46.07 | Ch3ck_ | will do. |
| 14:47.36 | Notify | 03BRL-CAD:carlmoore * 56580 brlcad/trunk/src/libbrep/intersect.cpp: OK, fixed 'minimum' spelling (although intersect.cpp got other, unrelated warning messages); did not find 'mininum' misspelling anywhere else |
| 14:48.32 | Ch3ck_ | starseeker: could you please explain to me what rt_db_get_internal does exactly? |
| 14:48.42 | Ch3ck_ | read the code but still have some blind spots. |
| 14:49.16 | Izak__ | starseeker: did you append the method for oriented bounding boxes to the rt_functab[] ? |
| 14:53.19 | *** join/#brlcad kesha_ (~kesha@14.139.122.114) |
| 14:53.38 | Notify | 03BRL-CAD:starseeker * 56581 (brlcad/trunk/src/other/libgdiam.dist =================================================================== and 10 others): Add distcheck file for gdiam |
| 14:56.41 | brlcad | kesha_: can you talk to me about patch 218? |
| 14:56.56 | brlcad | it's interesting, surprising even that you worked on those |
| 14:57.35 | brlcad | a quick glance, and I see that you (fortunately) limited yourself to c++ operations, which is good |
| 14:58.01 | brlcad | however, you do make a performance claim and this should be tested |
| 15:01.21 | brlcad | kesha_: and on testing, it doesn't apply cleanly |
| 15:06.38 | kesha_ | brlcad: whats the problem you are facing ? cleanly as in ? |
| 15:06.51 | ``Erik | with regard to performance: don't. Just, don't. Make it work before even contemplating performance, it's very easy to make bad assumptions and produce over-complicated code that actually runs slower... until you're an old super-guru, I'd strongly recommended clearing your minds of performance until after your code is correct and complete |
| 15:21.00 | brlcad | kesha_: patches should apply with "patch -p0 -i yourfile.patch" to a trunk checkout |
| 15:26.08 | brlcad | Ch3ck_: your patches are seriously getting out of control |
| 15:26.26 | brlcad | you need to stop creating new patches unless they're for a completely unrelated new change to code |
| 15:26.41 | Ch3ck_ | thats what i've been doing. |
| 15:26.51 | Ch3ck_ | making them unrelated |
| 15:27.10 | Ch3ck_ | but since they patch files all have to enter the same files |
| 15:27.26 | Ch3ck_ | problems may arise with CMakeLists |
| 15:27.40 | Ch3ck_ | since they're all trying to enter the same place |
| 15:27.55 | Ch3ck_ | I wanted to make it in sure a way they apply cleanly and consecutively |
| 15:28.09 | ``Erik | if two patches touch the same file, one of them will be bounced back for rework due to conflict... |
| 15:28.10 | Ch3ck_ | i was adviced to make them independent and thats what i did |
| 15:28.16 | Ch3ck_ | yeah |
| 15:28.20 | Ch3ck_ | thats the problem |
| 15:28.28 | Ch3ck_ | well if they all have to apply cleanly |
| 15:28.30 | brlcad | 190 and 217 seem the same, 206 and 215 seem the same |
| 15:28.35 | Izak__ | starseeker: Which commit did the oriented bounding box go in? |
| 15:28.49 | Ch3ck_ | i'll have to apply them consecutively |
| 15:28.58 | Ch3ck_ | so they clearly fit in the CMakeLists |
| 15:29.26 | Ch3ck_ | that should be the only problem with the patches. |
| 15:29.33 | ``Erik | also; stop hitting enter so much, try writing a good line instead of a lot of half-assed lines O.o |
| 15:29.34 | brlcad | Ch3ck_: if they are independent or if the ordering is in the patch, that's fine |
| 15:29.48 | Ch3ck_ | well |
| 15:29.54 | brlcad | i think the issue was just those four older ones seeming to be duplicates as I read down through the list |
| 15:29.59 | Ch3ck_ | star seeker told me to make them in such way that |
| 15:30.19 | Ch3ck_ | they should be applied in any order indepently |
| 15:30.26 | Ch3ck_ | thats when this problem is coming up. |
| 15:30.35 | Ch3ck_ | which ones? |
| 15:30.46 | brlcad | 11:28 < brlcad> 190 and 217 seem the same, 206 and 215 seem the same |
| 15:31.25 | Ch3ck_ | well they're not the same |
| 15:31.30 | Ch3ck_ | there are changes in them.. |
| 15:31.38 | Ch3ck_ | and should apply cleanly |
| 15:31.44 | brlcad | okay, so then just their subject titles are misleading... ;) |
| 15:32.00 | Ch3ck_ | probably its just the naming |
| 15:32.09 | Ch3ck_ | since all the patches are trying to enter at once |
| 15:32.16 | Ch3ck_ | i could combine all of them into one |
| 15:32.28 | Ch3ck_ | so they just apply neatly then.. |
| 15:32.29 | zero_level | brlcad : what did u mean here 14:33 < brlcad> zero_level: "facsq,x,y,py,px,c;" is your doing ;) |
| 15:32.45 | brlcad | Ch3ck_: and that would be bad (we've had this discussion before...) |
| 15:32.51 | Ch3ck_ | ok |
| 15:33.00 | Ch3ck_ | then in that case i'll have to make them consecutive |
| 15:33.05 | Ch3ck_ | so they apply cleanly |
| 15:33.08 | Ch3ck_ | is that ok? |
| 15:33.17 | brlcad | that is usually fine |
| 15:33.26 | Ch3ck_ | and i'll also have to move the comments of GNU octave inside |
| 15:33.27 | Ch3ck_ | the file |
| 15:33.36 | brlcad | the point is actually to resolve these patches NOW because this is not how we want you to work |
| 15:33.43 | Izak__ | starseeker: Which commit during the weekend had the oriented bbox changes ? |
| 15:33.50 | brlcad | patch files are only being used because there continue to be problems with the patch files |
| 15:33.55 | Ch3ck_ | i don't understnad |
| 15:34.11 | Ch3ck_ | should I fix all these now and submit? |
| 15:34.15 | brlcad | zero_level: there is something wrong there, what is it? |
| 15:34.16 | zero_level | Izak__ : I advise you to join brlcad-commits list |
| 15:34.36 | zero_level | to many variables in a single declaration ? |
| 15:34.46 | brlcad | Izak__: you can run svn log to see changes |
| 15:34.49 | zero_level | c/to/too |
| 15:34.53 | brlcad | zero_level: nope |
| 15:35.04 | brlcad | look at the original code, then look at yours |
| 15:35.09 | zero_level | alright. |
| 15:35.17 | brlcad | you changed something and it even goes against our HACKING guidelines |
| 15:36.06 | brlcad | it's minor but you should be able to see when you inject this sort of change as it just becomes cleanup work for someone else later, and that's not efficient use of time |
| 15:36.26 | zero_level | ah.. so u mean the gaps. ? |
| 15:36.28 | brlcad | Ch3ck_: no, lets go through them |
| 15:36.34 | Notify | 03BRL-CAD:tbrowder2 * 56582 (brlcad/trunk/src/conv/jack/g-jack.c brlcad/trunk/src/sig/c-d.c and 9 others): standardize usage strings for man-pageless progs; add optstring var for bu_getopts |
| 15:36.35 | Ch3ck_ | ok |
| 15:36.42 | Ch3ck_ | i'm listening |
| 15:36.56 | brlcad | 190 |
| 15:37.11 | Notify | 03BRL-CAD:mohitdaga * 56583 brlcad/trunk/src/libicv/decimate.c: fix extra identifier in the line. |
| 15:37.12 | brlcad | your last comment there was that you were going to resubmit an up-to-date patch |
| 15:38.02 | Ch3ck_ | for which patch exactly |
| 15:39.08 | brlcad | kesha_ a word of caution, I suggest breaking up your work be in slightly smaller chunks (for stepcode changes) because you keep creating too much work for mpictor_ |
| 15:40.01 | brlcad | kesha_: I realize you've reviewed and fixed some of these issues with the latest commits, but he shouldn't have to keep reiterating the same points |
| 15:40.32 | brlcad | kesha_: perhaps make a checklist of things to review when you make a commit to make sure that you don't create work for him to respond to other than "looks great" |
| 15:40.42 | brlcad | Ch3ck_: 190! |
| 15:42.18 | Ch3ck_ | well i had seen that as rejected |
| 15:42.24 | brlcad | kesha_: in that checklist should include doing a check to make sure the code compiles, reviewing the diff to make sure only intended and relevant changes are included, testing your change before commit (or using the dashboard) and after to make sure nothing was broken |
| 15:42.42 | brlcad | ~dict pending |
| 15:42.53 | Ch3ck_ | thats why i created the new patch 217 |
| 15:43.13 | brlcad | that is EXACTLY what I just said you need to stop doing :) |
| 15:43.23 | Ch3ck_ | I had told you that 190 had significant debug problems |
| 15:43.36 | Ch3ck_ | i thought with the pending-rejected comment |
| 15:43.37 | brlcad | if it's the same patch, you should have posted it to 190 instead of creating yet another tracker issue |
| 15:43.54 | Ch3ck_ | it means i should created a better one |
| 15:43.56 | Ch3ck_ | sorry |
| 15:43.57 | Ch3ck_ | my bad |
| 15:44.01 | Ch3ck_ | but its now 217 |
| 15:44.10 | brlcad | what does pending mean? |
| 15:44.15 | Ch3ck_ | I did not understand what pending-rejeced |
| 15:44.23 | Ch3ck_ | pending : still to be |
| 15:45.01 | brlcad | ~dict 2 pending |
| 15:45.34 | brlcad | "not yet decided" ... and if you do nothing, it will be rejected |
| 15:45.45 | Ch3ck_ | ok |
| 15:45.46 | brlcad | it was waiting for you to fix it |
| 15:45.58 | Ch3ck_ | and thats what i did with 217 |
| 15:46.04 | brlcad | that's not fixing 190 |
| 15:46.07 | brlcad | that's creating another issue |
| 15:46.11 | Ch3ck_ | yeha |
| 15:46.13 | kesha_ | brlcad: yes, Agree with you. the idea of creating a checklist looks great ! It will save much of mpictor_ 's and other time. |
| 15:46.17 | Ch3ck_ | so how do i remove 217 |
| 15:46.21 | brlcad | you don't |
| 15:46.28 | brlcad | we just have more work to clean this up |
| 15:46.32 | Ch3ck_ | ok |
| 15:46.36 | brlcad | which is bad |
| 15:46.40 | Ch3ck_ | let me move the changes here. |
| 15:46.41 | kesha_ | Regarding the patch of brlcad, I will get back soon. Right now I am working on STEP |
| 15:46.50 | brlcad | Ch3ck_: move what changes? |
| 15:47.05 | Ch3ck_ | the patch at 217 |
| 15:47.07 | Ch3ck_ | here |
| 15:47.11 | Ch3ck_ | 190 |
| 15:47.37 | brlcad | if 217 is a follow-on/duplicate/continuation of 190, I'll just close 190 and we can move forward |
| 15:48.19 | brlcad | but this is yet another failure in communication on both our parts, mine for making it clear, and yours for not understanding the status or impact of creating new submissions |
| 15:48.40 | brlcad | you should have asked what pending-rejected meant if you didn't understand |
| 15:49.24 | brlcad | there are plenty of language barriers we have to overcome, but we cannot if you do not ask for assistance or let someone know when something isn't 100% clear |
| 15:49.25 | Ch3ck_ | ok fine |
| 15:49.53 | Ch3ck_ | lets move on then.. |
| 15:51.37 | brlcad | Ch3ck_: so I'll ask again.... 206 and 215 seem the same |
| 15:51.39 | brlcad | are they? |
| 15:52.04 | Ch3ck_ | yes. |
| 15:53.49 | brlcad | okay, then it it safe to assuming 206 can be closed? |
| 15:53.54 | brlcad | s/assuming/assume/ |
| 15:54.42 | Ch3ck_ | yes |
| 15:54.46 | Ch3ck_ | it can be closed |
| 15:55.20 | Ch3ck_ | so all changes on push and pull will be on 215 and 217 |
| 15:55.27 | Ch3ck_ | henceforth |
| 15:56.28 | brlcad | okay ... and if anyone marks any of your patches as pending-* including pending-rejected ... that is not a closed issue, and you can/should update *that* issue #, not create a new one |
| 15:56.54 | brlcad | pending means we're waiting on you to do something |
| 15:57.31 | Ch3ck_ | ok |
| 15:57.40 | Ch3ck_ | I understand it perfectly now. |
| 16:00.39 | brlcad | kesha_: okay, let me know when you get back to it 1) so I can help make sure you're testing the change right and 2) to make sure you don't spend too much time on it |
| 16:00.52 | brlcad | kesha_: though I am curious why you're patch is out of sync in the first place |
| 16:02.48 | kesha_ | brlcad: okay. |
| 16:03.39 | kesha_ | brlcad: btw are you getting something like http://paste.kde.org/p8787aee0/ ? |
| 16:03.55 | brlcad | yep |
| 16:04.01 | brlcad | there should be no failures |
| 16:04.15 | brlcad | implies the patch was created with old sources |
| 16:04.32 | brlcad | so you'll want to go back to those sources, update them, and regenerate the patch file |
| 16:04.43 | brlcad | resolving any conflicts that arise during update |
| 16:05.09 | kesha_ | How to regenerate again ? I mean wont it be lost ? |
| 16:05.09 | Izak__ | brlcad:I included a NULL field in table.c for the oriented bounding box starseeker: added but my compiler cries fawl "excess elements in struct initializer" |
| 16:05.25 | brlcad | reading through 30+ patch rejection failures would be more work than doing it all over |
| 16:05.27 | zero_level | kesha_ : do svn up. |
| 16:05.45 | zero_level | and then svn diff |
| 16:06.00 | brlcad | kesha_: where did you make those modifications? |
| 16:06.07 | zero_level | if the file you are working has not been modified. there shld not be issues. |
| 16:06.08 | brlcad | to what sources? |
| 16:06.19 | Notify | 03BRL-CAD:starseeker * 56584 (brlcad/trunk/src/conv/step/ON_Brep.cpp brlcad/trunk/src/conv/step/g-step.cpp): note that STEPentity == SDAI_Application_instance |
| 16:06.39 | brlcad | Izak__: have you svn updated include/ |
| 16:07.10 | Izak__ | brlcad: I did an svn update before building |
| 16:07.57 | brlcad | excess elements means you've got too many fields, but I don't know the status of your code to know where exactly that is |
| 16:08.12 | brlcad | you can always count them, see what the header says |
| 16:08.36 | brlcad | make sure there's one and only one element listed for each struct element |
| 16:08.53 | Notify | 03BRL-CAD Wiki:NyahCh3ck20 * 5926 /wiki/User:NyahCh3ck20/GSoc2013/Coding_Repor: /* 5 August - 11 August */ |
| 16:09.16 | brlcad | it's very possible that svn update merged in the new field just fine, adding a new NULL element, and you adding another becomes too many |
| 16:09.59 | brlcad | Izak__: so I'm a little saddened that we don't have a successful outcome from patch 191 |
| 16:10.06 | brlcad | I really hate to see work go to waste |
| 16:10.58 | brlcad | all that's required is to show that your patch provides some improvement, ideally by demonstrating a case where rb_delete was wrong or failed, and with yours now it doesn't |
| 16:13.02 | Notify | 03BRL-CAD:starseeker * 56585 brlcad/trunk/src/conv/step/g-step.cpp: whoop, committed too many files |
| 16:14.49 | Izak__ | brlcad: Please, with all due respect, could we discuss the crisis on table.c first |
| 16:16.08 | kesha_ | brlcad: http://paste.kde.org/pbbd6a362/ |
| 16:16.08 | Izak__ | brlcad: This is the status of my table.c code here http://paste.kde.org/pb0c0bbcf/ |
| 16:16.11 | kesha_ | :) |
| 16:16.50 | Izak__ | kesha: what's with the :) ? |
| 16:17.35 | kesha_ | Izak__: http://paste.kde.org/pbbd6a362/ worked , I presume. |
| 16:18.16 | brlcad | starseeker: stray comma on ON_BRep_to_STEP ? |
| 16:18.19 | Izak__ | kesha: It didn't. Just showing Sean the status of some of my code :) |
| 16:18.55 | kesha_ | Izak__: I am talking abt <kesha_> brlcad: http://paste.kde.org/pbbd6a362/ . Anyways, leave it ! |
| 16:18.57 | brlcad | Izak__: the fact that 191 and other patches are unresolved is at the heart of why you have a table.c crisis |
| 16:19.24 | brlcad | if you had established commit access by now, you wouldn't be fighting painful merge issues like this |
| 16:20.15 | brlcad | so if you don't want to talk about it now, that's fine, but please do understand that this is not an efficient or preferred method of communication |
| 16:20.41 | brlcad | I'm reviewing patches ... so I don't have to review your patches |
| 16:20.51 | brlcad | so you don't have to create patches |
| 16:21.00 | Izak__ | brlcad: Wer can talk about it then. |
| 16:21.01 | brlcad | it's far FAR easier to review commits |
| 16:21.56 | brlcad | kesha_: is that to imply that you were able tol update your sources and regenerate a clean patch? |
| 16:22.11 | brlcad | if so, please do upload it and I'll give it a review |
| 16:22.29 | kesha_ | brlcad: ya, I am attaching new patch over there |
| 16:22.36 | Ch3ck_ | so sean what's the next step? |
| 16:23.07 | brlcad | Izak__: so as for table.c and the current definition of the function table, is that too many or too few struct elements? |
| 16:24.16 | brlcad | Ch3ck_: what do you mean? :) |
| 16:24.39 | Ch3ck_ | waiting on the erviews we were discussing |
| 16:24.48 | Ch3ck_ | reviews we were working on |
| 16:24.52 | brlcad | you mean patches 215 and 217? |
| 16:24.56 | Ch3ck_ | yeah.. |
| 16:25.53 | brlcad | I generally review patches in order when it pertains to commit access, and there are a few ahead of 215 now |
| 16:26.00 | brlcad | all the more reason to update your oldest patches... ;) |
| 16:26.24 | brlcad | i should get to it later today, so carry on with what you're doing |
| 16:26.34 | ``Erik | Izak__: please do 'svn info' and verify that your revision is 56585, you should be running 'svn up' fairly often |
| 16:26.35 | brlcad | thank you for clarifying those two |
| 16:26.42 | Ch3ck_ | ok |
| 16:26.46 | Ch3ck_ | what about the unit tests? |
| 16:26.55 | brlcad | they're even newer right? |
| 16:26.58 | brlcad | :) |
| 16:26.59 | Ch3ck_ | which i've written to get commit access |
| 16:27.02 | Ch3ck_ | yes. |
| 16:27.19 | brlcad | all the more reason to update/check/fix your oldest patches... ;) |
| 16:27.19 | Ch3ck_ | so since there are hunk fails during application |
| 16:27.30 | brlcad | otherwise it isn't fair to others |
| 16:27.33 | Ch3ck_ | of the patches |
| 16:27.43 | brlcad | yeah, you'll want to address that |
| 16:27.54 | Ch3ck_ | so should i make them in such a way that they apply consecutively and cleanly? |
| 16:27.58 | brlcad | yes |
| 16:28.02 | Ch3ck_ | ok |
| 16:28.07 | brlcad | or independently |
| 16:28.17 | brlcad | but then independent will apply consecutively and cleanly too |
| 16:28.52 | brlcad | just if there IS a dependency, leave a comment that simply says "this patch assumes patch #224 will be applied first" |
| 16:28.55 | ``Erik | Ch3ck_: if a patch depends on another, please note the relationship in the description... "this patch depends on 215" or something |
| 16:29.20 | Ch3ck_ | ok |
| 16:29.30 | brlcad | so I did read through your bn testing changes |
| 16:29.34 | brlcad | it's looking really good |
| 16:29.50 | brlcad | loving the inclusion of real outputs from octave |
| 16:30.02 | brlcad | only concern is that testing looked VERY limited in scope |
| 16:30.51 | brlcad | at least for the poly testing, didn't see any zero coefficients, negative values, large with small, small with large, and didn't go over fourth order, I believe |
| 16:31.29 | brlcad | you created the testing harness, so additional tests are nearly free to set up |
| 16:32.02 | brlcad | you can set up as many as you can think of that test some unique characteristic fairly easily to make a decently robust test |
| 16:33.07 | Ch3ck_ | yeah |
| 16:34.11 | brlcad | kesha_: I suggest going back through mpictor_'s github and email comments to you and make a note/list of things he said were wrong, then add those to your checklist |
| 16:39.40 | brlcad | ejno: I just created an opencl branch for you to work in, you can start integrating your work to date there including any sample/test code you're working on |
| 16:40.16 | ejno | ok, thank you |
| 16:41.03 | brlcad | i'd like to see if we can establish a real proof-of-concept as quickly as possible, so don't worry about doing it pretty |
| 16:41.26 | brlcad | we can treat this as a testing throw-away branch, and if a concept pans out, we can integrate it cleanly on trunk |
| 16:44.25 | ejno | ok |
| 16:44.29 | Notify | 03BRL-CAD:brlcad * 56586 NIL: Created an OpenCL branch for some experimental performance impact testing. Looking to feed the primitive evaluations through OpenCL for starters to evaluate the scalability and performance impact for a limited set of primitives. This is intended to be proof-of-concept work, not working production code nor any guarantees implied (including portable compilation). |
| 16:44.47 | brlcad | ``Erik: heh NIL .. nice :) |
| 16:45.26 | brlcad | ejno: https://svn.code.sf.net/p/brlcad/code/brlcad/branches/opencl |
| 16:45.33 | ejno | currently I'm having crashes when creating a new file in MGED. I've gone back 300 revisions also |
| 16:46.22 | brlcad | oof |
| 16:46.31 | brlcad | how are you creating a new file? |
| 16:46.53 | ejno | both File->New and on the command line: mged test.g |
| 16:47.10 | ``Erik | what about 'opendb newfile.g' at the mged prompt? |
| 16:47.40 | brlcad | hm, yeah, I dont' recall the last time I used the menu option |
| 16:48.11 | ``Erik | just created a new file doing "mged test.g" in bash... |
| 16:48.33 | ``Erik | and the menu |
| 16:48.59 | ejno | ``Erik: opendb works for me |
| 16:50.05 | ``Erik | Izak__: what's the progress on updating your table.c ? |
| 16:52.07 | ejno | this is printed: Fontconfig warning: FcPattern object fontfeatures does not accept value True |
| 16:52.09 | Izak__ | ``Erik: I am building and its succeeding..... Will update the patch soon. |
| 16:53.53 | Ch3ck_ | need some help here creating a patch |
| 16:54.16 | Ch3ck_ | I have created a patch and added a new test to brlcad |
| 16:54.39 | Ch3ck_ | and i want the new patch to have only the difference from the previously created patch? |
| 16:54.48 | Ch3ck_ | do i diff both patch files or what? |
| 16:55.33 | Ch3ck_ | since the new patch has both the previous files and the current one. |
| 16:57.34 | brlcad | Ch3ck_: "it depends" on what kind of changes you made, but ideally the file does only contain the new changes or is an update/replacement to the other patch |
| 16:58.01 | Ch3ck_ | i am creating patches consecutively |
| 16:58.04 | Ch3ck_ | on the tests |
| 16:58.18 | Ch3ck_ | and i want each to depend on the other consecutively since i'm adding |
| 16:58.23 | Ch3ck_ | tests |
| 16:58.26 | Ch3ck_ | to libbn |
| 16:58.37 | Ch3ck_ | when i create the patch it gives |
| 16:58.47 | Ch3ck_ | me both the previously added test and the current |
| 16:59.01 | Ch3ck_ | but i want it to have only the current |
| 16:59.07 | Ch3ck_ | but will apply when the previous has applie |
| 16:59.08 | Ch3ck_ | d |
| 16:59.18 | brlcad | yep, I get it |
| 16:59.54 | brlcad | without commit, it's fugly, so you might want to try and work on non-conflicting changes regardless so you can keep using "svn diff" |
| 17:00.01 | brlcad | otherwise, you'll probably need two checkout trees so you can run "diff -u" between them |
| 17:00.21 | brlcad | one dir is your previous state, other is your new state |
| 17:00.29 | brlcad | creates the delta patch |
| 17:01.07 | brlcad | I would try to minimize working like this as much as possible though |
| 17:01.11 | brlcad | we don't want you working that way |
| 17:01.31 | brlcad | perfect patches is the goal, and you only need a couple |
| 17:02.41 | Ch3ck_ | well i'll see what to do... |
| 17:02.47 | brlcad | so you could already have enough open patches to gain commit if they're perfect, in which case this is all moot (unless you already know they will not apply cleanly) |
| 17:03.12 | Ch3ck_ | to make independent patches apply independently cleanly |
| 17:03.16 | Ch3ck_ | but consecutively |
| 17:03.46 | ``Erik | Ch3ck_: how about you close the patch tickets that are no longer valid, pick one remaining patch and we try to get that one to a committable state... throwing crap patch after crap patch is just creating a big pile of crap... :D |
| 17:04.07 | Ch3ck_ | ok |
| 17:04.18 | Ch3ck_ | look at 222 |
| 17:04.23 | Ch3ck_ | it should apply cleanly |
| 17:04.29 | Ch3ck_ | so i move on to the next |
| 17:04.56 | brlcad | ejno: oh, now that you have commit, will you apply your 194 patch |
| 17:05.31 | ``Erik | 222 says it needs 221, is 221 going to be a clean good patch? |
| 17:05.41 | Ch3ck_ | 221 has already been applied |
| 17:05.46 | ``Erik | ah, cool |
| 17:05.47 | Ch3ck_ | so it should apply now. |
| 17:06.56 | ``Erik | the poly_init() function in that patch adds a trailing whitespace |
| 17:07.29 | Ch3ck_ | what! |
| 17:07.38 | ejno | brlcad: ok |
| 17:07.54 | ``Erik | and fails to apply cleanly... let me delete my copy and re-download to make sure I don't have an old one |
| 17:08.03 | brlcad | ejno: you can assign to yourself when done and mark it closed |
| 17:08.16 | ejno | ok |
| 17:08.37 | ejno | is it ok that I can't test the windows code? |
| 17:08.54 | brlcad | it's okay until you hear otherwise |
| 17:09.05 | *** join/#brlcad vladbogo (~vlad@188.25.239.5) |
| 17:09.17 | brlcad | if someone reports an issue, you'll need to address it or revert (usually resolve within 24 hours is expected) |
| 17:09.24 | ejno | ok |
| 17:09.28 | ``Erik | Ch3ck_: patches should be generated from the root directory of the project |
| 17:09.39 | brlcad | you can ask here or mailing list to see if someone is willing to test for you |
| 17:09.50 | Ch3ck_ | ok well generated a new one now.. |
| 17:11.09 | Ch3ck_ | updating the patch now.. |
| 17:11.43 | Ch3ck_ | check the updated version now. |
| 17:17.22 | ``Erik | ok, patch is generated correctly now and applies cleanly, but indentation is wrong (two space indentation is not what HACKING specifies), and there are two many empty lines |
| 17:18.45 | Ch3ck_ | wow |
| 17:18.50 | ``Erik | the end of test_bn_poly_scale() has a blank line between the return value and the closing curly, poly_init() has a blank line between the multiline comment about octave and the code it belongs to |
| 17:19.19 | ``Erik | most of these instances where you have two sequential blank lines should be reduced to one blank line |
| 17:19.33 | ``Erik | should never have a blank line after an opening curly |
| 17:20.37 | ``Erik | I have no idea what "o coefficients" are in the poly_init() comment, is that supposed to be '0' ? |
| 17:20.45 | Ch3ck_ | thats whay sean did with bn_poly_multiply |
| 17:20.52 | Ch3ck_ | so just followd |
| 17:21.14 | Ch3ck_ | yes 0 |
| 17:21.15 | ``Erik | comment line "*Initialises polnomial storing a negative, positive and zero coefficients." should have a space between * and I |
| 17:21.27 | ``Erik | and polynomial should be spelled correctly |
| 17:22.50 | ``Erik | in main(), the closing paren and else should be cuddled (on the same line, like "} else" ) |
| 17:23.10 | ``Erik | main() shouldn't use exit(), it should return the value instead |
| 17:23.46 | Ch3ck_ | i returned a negative number |
| 17:23.49 | Ch3ck_ | and sean refused |
| 17:23.55 | Ch3ck_ | prefering |
| 17:23.58 | Notify | 03BRL-CAD:ejno * 56587 brlcad/trunk/src/libbu/tests/CMakeLists.txt: add a test of libbu semaphore locking |
| 17:24.07 | Ch3ck_ | EXIT_FAILURE |
| 17:24.37 | ``Erik | ... you should use "return EXIT_SUCCESS;", not "exit(0);" ... |
| 17:25.11 | Ch3ck_ | thats what i did |
| 17:25.23 | Ch3ck_ | exit(EXIT_SUCCESS |
| 17:25.25 | Ch3ck_ | ) |
| 17:25.38 | Ch3ck_ | ok |
| 17:25.41 | Ch3ck_ | i get it.. |
| 17:25.53 | ``Erik | the string 'exit' should not exist in your program at all :) |
| 17:26.29 | Ch3ck_ | is that all |
| 17:26.36 | Ch3ck_ | so I should generate patch now? |
| 17:27.03 | ``Erik | and towards the top, the comment block about octave needs spaces between the '*' and body... I'd actually remove the copyright and build info from that comment as well, just say it was Octave 3.4.3 |
| 17:27.26 | Ch3ck_ | ok |
| 17:27.30 | Notify | 03BRL-CAD:starseeker * 56588 brlcad/trunk/src/conv/step/ON_Brep.cpp: Stray comma (thanks Sean) |
| 17:28.05 | ``Erik | after all that, regenerate and resubmit, and we'll take another look to see if anything else stands out, I guess |
| 17:29.00 | mpictor_ | kesha_: if you don't have a list of my comments, I believe I saved the issue 21 html file. The comments are tied to the commit id, so they disappear if the commit is removed |
| 17:29.29 | Ch3ck_ | ``Erik: should see new patch now.. |
| 17:30.36 | ``Erik | still indented wrong |
| 17:31.14 | Ch3ck_ | I ran it through indent.sh |
| 17:31.15 | Ch3ck_ | first |
| 17:31.20 | ``Erik | and too many blank lines |
| 17:31.23 | Ch3ck_ | So i don't understand |
| 17:31.55 | Ch3ck_ | i also did ws.sh |
| 17:32.08 | ``Erik | indent.sh depends on having brlcad's .emacs file/version/etc, it's not a magic hammer... (I've never gotten it to work right) |
| 17:32.28 | ``Erik | what is your text editor of choice? |
| 17:32.34 | Ch3ck_ | well i use gedit |
| 17:32.38 | Ch3ck_ | and emacs |
| 17:32.47 | Ch3ck_ | but i've not used emacs for some time now |
| 17:32.57 | Ch3ck_ | so i've forgotten many of its commands. |
| 17:34.44 | ``Erik | hm, gedit has naive indentation and I d'no the emacs fu to load the footer :/ |
| 17:36.03 | Ch3ck_ | so we do we do now.. |
| 17:36.14 | Ch3ck_ | i've done everything within my power.. |
| 17:36.19 | Ch3ck_ | i don't understand all these complications. |
| 17:36.22 | Ch3ck_ | :( |
| 17:36.37 | ``Erik | I'm googling the appropriate things to fix/add in your ~/.emacs file |
| 17:37.03 | Izak__ | ``Erik: I have upgraded the 'bare bones' patch |
| 17:38.46 | ``Erik | add (setq c-basic-offset 4) to your ~/.emacs and try the indent.sh script again, then visually inspect to see if the code is indented with 4 spaces? |
| 17:39.16 | Ch3ck_ | ok |
| 17:41.00 | Ch3ck_ | where is the ~/.emacs found |
| 17:41.25 | ``Erik | ~/ means in your home directory... you can run "emacs ~/.emacs" from anywhere |
| 17:42.37 | Ch3ck_ | so how do i add the setq c-ba ... |
| 17:42.39 | Ch3ck_ | thing.. |
| 17:42.44 | *** join/#brlcad kesha__ (~kesha@14.139.122.114) |
| 17:42.58 | ``Erik | go to the bottom of the file, type (or paste) it, save and exit... |
| 17:44.14 | Ch3ck_ | i have done that within the bn_poly_scale.c file |
| 17:45.04 | ``Erik | this (setq c-basic-offset 4) line should not be in bn_poly_scale.c, it should be in your .emacs file |
| 17:46.22 | Ch3ck_ | i have not seen any .emacs file |
| 17:47.16 | ``Erik | it'd be a hidden file in your home directory... if it doesn't exist, you have to create it... |
| 17:48.51 | Ch3ck_ | how should i save it |
| 17:48.55 | Ch3ck_ | as .emacs? |
| 17:49.01 | Ch3ck_ | and what.emacs |
| 17:50.12 | Ch3ck_ | ok did it |
| 17:50.19 | Ch3ck_ | but there is still no change |
| 17:50.24 | Ch3ck_ | should i upload the patch? |
| 17:56.22 | *** join/#brlcad kesha__ (~kesha@14.139.122.114) |
| 18:01.32 | ``Erik | Ch3ck_: if the patch still has 2 space indentation, no. |
| 18:02.08 | ``Erik | the ".emacs" file should be in your home directory |
| 18:02.20 | Ch3ck_ | thats what i've done |
| 18:02.20 | ``Erik | you should be able to do "cat ~/.emacs" and see the c-basic-offset line |
| 18:03.13 | ``Erik | (brlcad should cook a .indent.pro for use with bsd or gnu indent *cough*) |
| 18:04.35 | Ch3ck_ | well |
| 18:04.40 | Ch3ck_ | i've done it |
| 18:04.44 | Ch3ck_ | and will upload patch now. |
| 18:05.53 | Notify | 03BRL-CAD:ejno * 56589 brlcad/branches/opencl/src/librt/primitives/ell/ell.c: convert to C and integrate with librt |
| 18:06.12 | Ch3ck_ | should check patch now.. |
| 18:10.00 | ``Erik | well, until we figure out how to indent correctly, I'll point out some other things... like the blank lines y ou need to delete, I'll list them from bottom to top so the line numbers don't muddle things... 143 73 49 37 |
| 18:10.15 | ``Erik | also; copyright date says 2004-2013, that should just be 2013 since this file is new |
| 18:12.17 | Ch3ck_ | is that all? |
| 18:13.06 | ``Erik | other than indentation, that's all that's jumping out to me |
| 18:13.36 | ``Erik | do you have gnu's "indent" program on your machine? |
| 18:14.45 | brlcad | running indent.sh is a crutch, it's for convenience -- you should be able to do the whitespace corrections manually if needed |
| 18:14.59 | brlcad | there is an example .emacs file on the wiki |
| 18:15.16 | Ch3ck_ | ok |
| 18:15.26 | Ch3ck_ | please i'll need link |
| 18:15.42 | brlcad | how many lines are in your patch file? |
| 18:16.30 | brlcad | the "wc" program will tell you |
| 18:17.58 | Ch3ck_ | 191 |
| 18:18.31 | Ch3ck_ | now 187 |
| 18:18.32 | starseeker | Ch3ck_: uh... just use google - BRL-CAD and Emacs |
| 18:18.34 | brlcad | that should take less than two minutes to manually fix |
| 18:18.48 | Ch3ck_ | how many white spaces |
| 18:18.49 | Ch3ck_ | 4? |
| 18:18.54 | Ch3ck_ | right? |
| 18:18.57 | brlcad | no |
| 18:18.58 | starseeker | check HACKING |
| 18:19.09 | Ch3ck_ | ok |
| 18:20.00 | Ch3ck_ | indents every 4 characters |
| 18:20.12 | Ch3ck_ | tab stops at 8 characters. |
| 18:20.14 | Ch3ck_ | right? |
| 18:20.20 | Ch3ck_ | its what i'm reading. |
| 18:20.30 | brlcad | that's what it says, yes .. but do you understand what that means? |
| 18:21.06 | brlcad | it means it can be a mix of tabs and spaces for any given line, depending on how many indentation levels there are |
| 18:22.02 | brlcad | ``Erik: does sasl have an update or do you know if it updated recently? |
| 18:22.23 | brlcad | getting a fuckton of "svn: DIGEST-MD5 common mech free" messages now |
| 18:22.31 | brlcad | via syslog |
| 18:23.17 | brlcad | didn't just start happening, it's been a couple weeks |
| 18:23.23 | Ch3ck_ | i've manually made the indentation to 4 |
| 18:23.27 | vladbogo | hi all. I am currently working at mouse integration between qt and tk and I am dealing with a really strange problem: I've managed to send events from qt to tk and when I click I get the error "bad scaling factor: 0.000". |
| 18:23.44 | Notify | 03BRL-CAD:ejno * 56590 (brlcad/branches/opencl/test-raytracer/version1/Makefile =================================================================== and 32 others): add original code to SVN |
| 18:23.49 | brlcad | Ch3ck_: do you understand what is meant by saying it's a mix of tabs and spaces? |
| 18:24.00 | Ch3ck_ | no |
| 18:24.23 | vladbogo | apparantly this comes from "atof" function because the string is 0.5 and the result is 0 |
| 18:24.27 | brlcad | it's critical to knowing how to indent properly |
| 18:24.44 | brlcad | Ch3ck_: see one of the existing files that has multiple levels of indentation |
| 18:24.57 | Ch3ck_ | ok |
| 18:25.10 | brlcad | it's 4spcs, then 1tab, then 1tab+4spcs, then 2 tabs, etc |
| 18:25.41 | Ch3ck_ | i don't understand |
| 18:25.42 | brlcad | each indent either adds 4 spaces or removes four spaces and inserts a tab |
| 18:26.24 | brlcad | it's a byte-compressed fixed-presentation code style |
| 18:26.26 | brlcad | it's not just spaces |
| 18:26.28 | brlcad | it not just tabs |
| 18:26.30 | vladbogo | the strange part is that when I use any other dm insted of the Qt one the separator for double is "."(point) but for the qt one is ","(comma) so if I give to atof ("0,5") the conversion is correct. Any idea why this behaviour? |
| 18:27.12 | Izak__ | make |
| 18:27.19 | brlcad | vladbogo: atof("0.5") returning 0.0 sounds wrong |
| 18:28.24 | brlcad | either the string it gets is not actually "0.5" or it's not really returning 0.0 and somehow getting cast/converted somewhere |
| 18:28.54 | brlcad | e.g., without a prototype, the compiler will assume atof() returns an integer, so you have to make sure header is properly included |
| 18:29.23 | vladbogo | brlcad: I know and if I change the atof("0.5") to atof("0,5") (here it's comma) work's fine |
| 18:29.49 | ``Erik | starseeker: beginnings of an .indent.pro might be something like -bad -bap -bbb -br -ce -di1 -i4 -pcs -nfcb |
| 18:30.11 | ``Erik | brlcad: may've been updated... I've been doing port upgrades almost daily |
| 18:30.19 | brlcad | vladbogo: o.O |
| 18:30.20 | vladbogo | brlcad: and also when I print a double it uses the comma as a separator but just for the Qt dm |
| 18:30.44 | ``Erik | ah, vladbogo, do you have locale stuff set? |
| 18:30.46 | brlcad | "The decimal point character is defined in the program's locale (category LC_NUMERIC)" |
| 18:31.13 | brlcad | that's from atof's manual page |
| 18:31.20 | ``Erik | a lot of the world uses a comma instead of a period as a decimal point |
| 18:31.33 | vladbogo | ''Erik: I haven't set any locale explicitly |
| 18:32.03 | vladbogo | and for the other dm's it uses point as expected not comma |
| 18:32.42 | brlcad | vladbogo: you almost certainly have a locale set in your environment |
| 18:33.08 | brlcad | which means most of the standard functions are going to expect a comma |
| 18:33.39 | *** join/#brlcad kesha__ (~kesha@14.139.122.114) |
| 18:35.51 | brlcad | we may need to add setlocale( LC_ALL, "en-US" ); where we assume us locale, but that's certainly not elegant or polite |
| 18:42.26 | vladbogo | brlcad: should I try to set this somewhere specific or doesn't matter because nothing changes? |
| 18:48.36 | Notify | 03BRL-CAD:ejno * 56591 brlcad/branches/opencl/src/librt/primitives/ell/ell.c: more work on integrating with librt |
| 18:52.37 | brlcad | vladbogo: where's the 0.5 string coming from? |
| 18:54.22 | brlcad | Ch3ck_: also /*this is bad*/ and /* this is good */ when it comes to comments |
| 18:55.40 | brlcad | the committed sources need many style corrections, but to do so with several open patches would almost certainly introduce conflicts or make merging more difficult |
| 18:56.33 | vladbogo | brlcad: it's an argument to the cmd_zoom function |
| 18:56.53 | brlcad | file:line? |
| 18:56.55 | Ch3ck_ | brlcad: i don't have any such problems with patch 221 |
| 18:57.16 | brlcad | Ch3ck_: okay, just commenting based on current status of bn_poly_multiply.c |
| 18:57.34 | brlcad | function return types belong on separate line |
| 18:57.40 | brlcad | int |
| 18:57.43 | brlcad | main() |
| 18:57.46 | brlcad | { |
| 18:57.55 | vladbogo | mged/chgview.c line 2690 |
| 18:57.56 | brlcad | not int main() { |
| 18:58.34 | vladbogo | also I tested on my other system where I have kubuntu and it does not seem to have this behaviour |
| 18:58.53 | Ch3ck_ | waiting for the application of 221 |
| 18:59.01 | Ch3ck_ | so i could get on to the next patch |
| 18:59.04 | vladbogo | so it might be something local |
| 18:59.46 | ``Erik | brlcad: rebuilt subversion without sasl, that should eliminate the debug message... |
| 18:59.49 | ``Erik | heads home O.o |
| 18:59.51 | brlcad | vladbogo: locale is set in your environment |
| 18:59.58 | brlcad | ``Erik: you rock! |
| 19:00.10 | zero_level | hi all : Can every one run regress on there local machines. |
| 19:00.31 | zero_level | make regress in the build directory. |
| 19:00.49 | brlcad | vladbogo: if you run "set", there's almost certainly a locale setting in there |
| 19:01.41 | brlcad | vladbogo: so chgview.c:2690 is where the atof call was made, but where'd the 0.5 string come from? |
| 19:01.50 | brlcad | that came into cmd_zoom() from somewhere else |
| 19:02.37 | brlcad | usually user input, so a fix would be to type "zoom 0,5" in mged and that might work |
| 19:02.54 | brlcad | but then if we print 0.5 to a string or manually pass it in from some tclscript, we have a locale bug |
| 19:03.32 | brlcad | zero_level: anything more specific, I run that throughout the day every day |
| 19:03.33 | vladbogo | brlcad: there I found that the problem is: it occurs when the mouse button is pressed |
| 19:03.47 | brlcad | mouse button is a zoom in/out binding |
| 19:03.59 | zero_level | solids.sh |
| 19:04.00 | brlcad | so probably what I just said, a tclscript passing in 0.5 ignorantly |
| 19:04.44 | brlcad | zero_level: just fyi, "make regress-solids" will run just the solids regression (and it currently passes for me) |
| 19:04.53 | zero_level | alright. |
| 19:06.11 | zero_level | BZ also passes for me. |
| 19:06.39 | zero_level | but i recieve the following on ubuntu12.04 i3 |
| 19:06.41 | zero_level | http://paste.kde.org/pb2722cd3/ |
| 19:06.47 | brlcad | vladbogo: LANG, LC_MESSAGES, and LC_ALL are the environment variables that control locale, if you unset them .. then mged will work (but the locale bug still will exist obviously) |
| 19:06.58 | vladbogo | brlcad: mged/buttons line 366 appears to call mged_zoom(0.5) |
| 19:07.15 | brlcad | zero_level: that's not the actual failure, you need to include more output |
| 19:08.22 | zero_level | brlcad : how abt this |
| 19:08.24 | zero_level | http://paste.kde.org/p019f0e1e/ |
| 19:08.37 | brlcad | vladbogo: it's still numeric there, that's fine |
| 19:09.06 | brlcad | looks like chgview.c:2630 is where it gets converted to string |
| 19:09.10 | vladbogo | brlcad: I'll try to find where the variables are set and see why this only happens for Qt |
| 19:09.18 | brlcad | so there is where it needs to take locale into account |
| 19:09.37 | brlcad | Qt is very locale aware |
| 19:10.12 | brlcad | C's standard library has different function calls to get locale behavior |
| 19:11.38 | brlcad | so snprintf() is probably just turning it into "0.5" or "0,5" and someone down the line is confused |
| 19:11.50 | brlcad | vladbogo: try adding this before the snprintf just to test: setlocale(LC_ALL, "POSIX"); |
| 19:14.05 | vladbogo | brlcad: setting this uses point |
| 19:15.10 | vladbogo | so should I set this in the dm-qt just to be sure? |
| 19:15.18 | brlcad | nope |
| 19:15.51 | vladbogo | ok |
| 19:16.04 | brlcad | we're not really well tested for locale aware applications so a couple thoughts come to mind |
| 19:16.19 | brlcad | 1) should fix the actual crash, to prevent that from happening regardless |
| 19:16.24 | brlcad | where was it crashing? |
| 19:16.59 | vladbogo | libged/zoom/zoom.c line 64 |
| 19:17.28 | brlcad | okay so not crashing, just reporting an error |
| 19:17.59 | brlcad | what was the string that sscanf was scanning from? |
| 19:18.02 | vladbogo | yep |
| 19:18.49 | brlcad | "0,5" or "0.5" (prior to adding setlocal()) |
| 19:19.07 | vladbogo | in zoom.c it's already 0.0 |
| 19:19.19 | vladbogo | before it was "0.5" |
| 19:19.45 | brlcad | the argv[] is "0.0" or "0,0" ? |
| 19:21.39 | vladbogo | "0,000" |
| 19:21.44 | brlcad | i'm not yet seeing where this goes wrong |
| 19:21.49 | brlcad | that's interesting |
| 19:22.10 | brlcad | so ged_zoom() has argv[]="0,000" |
| 19:22.26 | vladbogo | it goes wrong several steps before |
| 19:22.38 | vladbogo | yes argv[1]="0,000" |
| 19:22.48 | vladbogo | which seems to be ok |
| 19:22.49 | brlcad | which implies chgview.c:2633 set "0,000", which implies the snprintf() returned that |
| 19:22.54 | vladbogo | yes |
| 19:23.16 | vladbogo | the problem occurs in cmd_zoom |
| 19:23.30 | vladbogo | there argv[1] is "0.5" |
| 19:24.22 | vladbogo | which is converted to double and sent do mged_zoom where it's converted back to string using snprintf |
| 19:25.10 | vladbogo | so it seems that the call to cmd_zoom seems to be the problem |
| 19:28.09 | brlcad | yeah, I think you pinned it |
| 19:28.32 | vladbogo | and in mged/setup.c Tcl_CreateCommand creates a comand that has the function cmd_zoom |
| 19:28.53 | vladbogo | so it has to be in Tcl/Tk I guess |
| 19:32.32 | brlcad | the 0.5 string is almost certainly coming from tcl |
| 19:32.40 | brlcad | and it's respecting your locale, so that's a 0 |
| 19:33.24 | Notify | 03BRL-CAD:brlcad * 56592 brlcad/trunk/src/mged/chgview.c: make sure the zoom string is sane so we don't propagate a zero |
| 19:33.42 | vladbogo | I think I found out |
| 19:33.53 | vladbogo | src/libtclcad/tclcad_obj.c |
| 19:34.11 | vladbogo | line 5156 |
| 19:34.25 | vladbogo | but I'm not sure |
| 19:35.09 | Notify | 03BRL-CAD:brlcad * 56593 brlcad/trunk/src/mged/chgview.c: avoid propagating a zero zoom to libged |
| 19:38.36 | brlcad | vladbogo: looks like a possibility, did a stack tace get you there? |
| 19:39.11 | vladbogo | no. I've searched for zoom |
| 19:39.28 | brlcad | ah |
| 19:39.31 | brlcad | more than likely it's src/tclscripts/mged/bindings.tcl |
| 19:39.43 | brlcad | since that's where the key binding for zoom in/out are set for mged |
| 19:39.46 | vladbogo | but it doesn't seem to be there |
| 19:39.52 | brlcad | archer uses libtclcad |
| 19:40.18 | brlcad | src/tclscripts/mged/openw.tcl also probably has a binding |
| 19:40.38 | brlcad | (for the menu zoom options) |
| 19:40.57 | brlcad | yep, sure enough both places |
| 19:41.52 | brlcad | so the fix is probably specific to the tcl hook functions |
| 19:42.22 | brlcad | they need to forcibly set the locale to POSIX, do their work, then set it back |
| 19:43.11 | brlcad | OR we some way to manage every possible floating point literal in a tcl script in locale form |
| 19:43.43 | brlcad | like "zoom [clocale 0.5]" |
| 19:46.01 | vladbogo | so should I set the locale in cmd_zoom to POSIX then change it back? |
| 19:46.53 | Notify | 03BRL-CAD:carlmoore * 56594 (brlcad/trunk/doc/docbook/system/man1/en/g-acad.xml brlcad/trunk/src/conv/g-acad.c): disuss the -P option, implement 'RTD.debug = |
| 19:46.56 | brlcad | that would be the first option I mentioned |
| 19:47.08 | brlcad | forcibly setting the locale |
| 19:47.21 | starseeker | brlcad: is http://viennacl.sourceforge.net/ of any interest for the opencl stuff? |
| 19:47.32 | brlcad | the issue is just one of user expectation ... vladbogo would you expect to be able to run "zoom 0.5" on the command line |
| 19:47.38 | brlcad | or would you expect "zoom 0,5" |
| 19:48.02 | vladbogo | well I'd expect to run zoom 0.5 |
| 19:48.51 | brlcad | starseeker: if/when we get to converting a primitive that uses those types of solvers, perhaps |
| 19:49.20 | vladbogo | I'm still confused why the locale is set to something else since my local configurations are to use "." |
| 19:49.45 | brlcad | "."? |
| 19:49.49 | brlcad | that's not a valid locale :) |
| 19:50.04 | brlcad | set | grep L |
| 19:51.22 | brlcad | vladbogo: if you expect that, then I'll just tell any future complaining users that it was your fault ;) |
| 19:51.28 | brlcad | certainly sounds fine to me |
| 19:51.40 | brlcad | but then that's the only locale I use, POSIX ftw |
| 19:52.26 | vladbogo | I agreed with you |
| 19:52.51 | brlcad | I know :) |
| 19:53.04 | brlcad | just saying... someone will probably eventually complain ;) |
| 19:53.15 | brlcad | and then they can make an awesome patch! |
| 19:53.20 | vladbogo | :) |
| 19:55.33 | Notify | 03BRL-CAD Wiki:IIIzzzaaakkk * 5927 /wiki/User:Izak/GSOC_2013_logs: /* Mid-term Evaluation week */ |
| 19:55.48 | ``Erik | heh, "my native language is C" |
| 19:57.57 | vladbogo | brlcad: I also have a question regarding the mouse integration |
| 19:58.10 | Notify | 03BRL-CAD:brlcad * 56595 brlcad/trunk/src/mged/chgview.c: test yer compiles igit |
| 19:59.15 | vladbogo | I was thinking about doing something like this: send a tk event when I get a Qt one |
| 19:59.42 | vladbogo | but the problem is when using a blocking event_check |
| 20:00.23 | vladbogo | because the dm blocks waiting for a Tk event |
| 20:01.27 | Notify | 03BRL-CAD:brlcad * 56596 brlcad/trunk/src/mged/mged.c: force POSIX locale since our tcl script infrastructure expects it. we could get away with just performing locale restrictions within the tcl callback functions, but right now that is several hundred potential functions where we'd be introducing redundant code. another possibility might be to introduce a tcl command like 'zoom [clocale 0.5]' so the scripts |
| 20:01.29 | Notify | themselves become aware, and that would probably be best and a lot of work. |
| 20:01.47 | brlcad | vladbogo: that should do the trick for now |
| 20:02.00 | vladbogo | thanks |
| 20:02.28 | brlcad | we'll need something more for archer, but that might become moot when the two are merged |
| 20:03.16 | vladbogo | so since clicking triggers a Qt event this should be processed, but instead everithing is blocked waiting for a Tk event |
| 20:04.15 | vladbogo | if I force everything to be non-blocking it works fine but I don't think this is the best approach |
| 20:05.46 | vladbogo | another idea I have which I haven't tested but it has to work is to make all the Tk events non-blocking for the Qt display manager and instead wait for a Qt event to trigger |
| 20:05.52 | vladbogo | would this approach be ok? |
| 20:06.30 | brlcad | starseeker: their benchmarks are interesting .. if i'm reading it right, you have to have about 10,000 to 1,000,000 values to work on for their implementation to show a gain |
| 20:07.13 | brlcad | starseeker: that's a bit nuts .. huge matrices, maybe for how you were attempting to sparse solve the bot2nurbs fit (but then you know my other ideas there too) |
| 20:07.57 | brlcad | vladbogo: i don't have enough information to say yay or nay to that |
| 20:08.18 | brlcad | vladbogo: on the surface, it sounds fine, even if everything were non-blocking or just tk |
| 20:08.31 | brlcad | it's not clear that the implication would be just yet |
| 20:09.26 | vladbogo | brlcad: I haven't figured out yet when blocking/non-blocking event check is called since every iteration depends on the previous iteration's result |
| 20:09.33 | Notify | 03BRL-CAD:ejno * 56597 brlcad/branches/opencl/src/librt/primitives/ell/ell.c: work on sphere intersection |
| 20:12.18 | brlcad | ejno: you closed the patch, but where's the commit? |
| 20:12.40 | vladbogo | but I don't think having blocking Tk process events it's possible: the only way I think is that when adding the Qt event in the event queue also a Tk event should be generated which I think could be just by modifying the Qt code |
| 20:12.49 | brlcad | vladbogo: you'll also have to carefully consider both mged and archer as their run loops are quite different |
| 20:13.26 | vladbogo | I have just focused on mged |
| 20:13.39 | Notify | 03BRL-CAD:starseeker * 56598 (brlcad/trunk/src/librt/CMakeLists.txt brlcad/trunk/src/other/libgdiam/CMakeLists.txt brlcad/trunk/src/other/libgdiam/gdiam.hpp): Untested, and almost certainly not right yet, but start working on the Windows DLL logic for gdiam. |
| 20:14.26 | vladbogo | I'll try then also on archer but since I haven't done any significant modification to the run loop (just called the qt processEvents function it shouldn't be a problem) |
| 20:17.24 | starseeker | brlcad: ah, yeah, probably not worthwhile then |
| 20:17.51 | starseeker | pity - was hoping we could leverage some work by other dedicated GPU/OpenCL researchers |
| 20:23.52 | starseeker | notes the BRL-CAD cmake overview PDF is up |
| 20:25.44 | vladbogo | brlcad: even though forcing locale to POSIX as soon as dm_open is called and Qt is used the locale is reset so it has to be forced after opening the dm to take effect |
| 20:26.14 | vladbogo | should I change this? |
| 20:36.15 | Notify | 03BRL-CAD:starseeker * 56599 brlcad/trunk/src/other/libgdiam/CMakeLists.txt: Copy/paste strikes again. |
| 20:40.20 | Notify | 03BRL-CAD:vladbogo * 56600 brlcad/trunk/src/mged/mged.c: Force POSIX locale only after the display manager is opened so that the changes take effect. |
| 20:41.22 | brlcad | starseeker: this isn't a problem of libraries |
| 20:42.16 | brlcad | we already have the code |
| 20:43.03 | brlcad | it's a matter of translating from C to OpenCL (very easy), using different data containers (easyish), and restructuring the order of calculations (some work) |
| 20:43.57 | brlcad | ejno: never mind, I found the commit .. just overlooked |
| 20:49.17 | brlcad | vladbogo: whose resetting the locale? |
| 20:49.37 | vladbogo | something I guess |
| 20:49.39 | brlcad | s/whose/who's/ |
| 20:49.48 | vladbogo | in Qt * |
| 20:50.19 | vladbogo | all I know is that after the qt_open is called the locale resets |
| 20:52.09 | vladbogo | brlcad: is there anywhere I can found all key bindings? |
| 20:52.42 | brlcad | yeah, it's qt's backend: https://github.com/matplotlib/matplotlib/issues/1867 |
| 20:53.31 | brlcad | so we could also just unset LC_ALL/LANG at program start |
| 20:53.43 | brlcad | your move to put the init after Qt initialization is fine for now |
| 20:54.10 | brlcad | vladbogo: a key binding can be created anywhere, so no .. but most for mged are in src/tclscripts/bindings.tcl |
| 20:54.35 | brlcad | you can easily create bindings on the fly in tcl |
| 20:55.16 | vladbogo | so when it comes to integrating the keyboard what should it be supported? |
| 21:00.50 | Notify | 03BRL-CAD:vladbogo * 56601 (brlcad/trunk/include/dm-qt.h brlcad/trunk/src/libdm/dm-qt.cpp): Send Tk button press events when Qt mouse events occur. |
| 21:03.44 | vladbogo | brlcad: is it ok if I put a #ifdef DM_QT "process events nonblocking" for the moment and then try to have a better solution? |
| 21:11.53 | Notify | 03BRL-CAD:vladbogo * 56602 brlcad/trunk/src/mged/dm-qt.c: Implemented the qt_doevent function. |
| 21:12.34 | Notify | 03BRL-CAD:vladbogo * 56603 brlcad/trunk/src/libdm/dm-qt.cpp: Removed debug info. |
| 21:13.21 | brlcad | vladbogo: sure |
| 21:13.47 | brlcad | key bindings are libdm's greatest limitation because originally the API was not at all concerned with key events |
| 21:13.52 | brlcad | the front-end application handled them |
| 21:14.13 | brlcad | most of the older devices didn't even support the notion, so you were left with console I/O |
| 21:14.47 | vladbogo | I see |
| 21:15.30 | vladbogo | and in this case I guess a generic way to send different key events from Qt to Tk would be best |
| 21:16.53 | brlcad | libdm should have some awareness of key bindings / events |
| 21:17.00 | brlcad | that might be the method to bridge them |
| 21:17.39 | vladbogo | I'll think about it and i'll come with a suggestion in the next days |
| 21:17.45 | brlcad | sounds good |
| 21:18.27 | brlcad | you'll find two systems currently in place for mged |
| 21:19.13 | brlcad | there's the low-level C interface that is used when one runs mged -c, and there's the tcl/tk bindings, which already mentioned are predominantly src/tclscripts/bindings.tcl |
| 21:19.41 | brlcad | ideally, none of the bindings would involve Tk (other than maybe as an event pass-through to C) |
| 21:19.55 | Notify | 03BRL-CAD:vladbogo * 56604 brlcad/trunk/src/mged/dm-qt.c: Implemented and added the cmd_hook to the display manager. |
| 21:20.01 | brlcad | we want to eventually replace Tk with Qt completely |
| 21:21.29 | vladbogo | ok, then I'll have also this in mind |
| 21:31.17 | brlcad | if you spent the rest of summer just getting that working, you would be a god |
| 21:31.22 | Notify | 03BRL-CAD Wiki:Vladbogolin * 5928 /wiki/User:Vladbogolin/GSoC2013/Logs: |
| 21:34.25 | Notify | 03BRL-CAD Wiki:Vladbogolin * 5929 /wiki/User:Vladbogolin/GSoC2013/Logs: /* Week 8 */ |
| 21:40.23 | Notify | 03BRL-CAD:vladbogo * 56605 brlcad/trunk/src/mged/mged.c: If the qt display manager is enabled, check for events non-blocking. |
| 21:40.41 | Notify | 03BRL-CAD Wiki:Vladbogolin * 0 /wiki/File:Tor1.png: |
| 21:41.00 | Notify | 03BRL-CAD Wiki:Vladbogolin * 0 /wiki/File:Tor2.png: |
| 21:42.49 | Notify | 03BRL-CAD Wiki:Vladbogolin * 5932 /wiki/User:Vladbogolin/GSoC2013/Logs: /* Week 8 */ |
| 22:03.47 | Notify | 03BRL-CAD:starseeker * 56606 brlcad/trunk/src/other/libgdiam/gdiam.hpp: Just use the std min and max... |
| 22:13.41 | Notify | 03BRL-CAD:brlcad * 56607 brlcad/trunk/src/util/bwshrink.c: calling bu_free() after malloc() is wrong, but we should be calling bu_malloc() so do that instead. added some basic input sanity checking. |
| 22:24.18 | Notify | 03BRL-CAD:starseeker * 56608 (brlcad/trunk/src/other/libgdiam/CMakeLists.txt brlcad/trunk/src/other/libgdiam/gdiam.cpp brlcad/trunk/src/other/libgdiam/gdiam.hpp): Closer - buildings with MSVC, but the obb operation crashes with an error having to do with a sort call using a CompareByAngles comparison - Expression: invalid operator< |
| 22:28.53 | starseeker | suspects MSVC is being picky about something that gcc is letting slide... |
| 22:29.43 | starseeker | gdiam.cpp:1651 |
| 22:30.19 | starseeker | correction, gdiam.cpp:1636 now |
| 22:54.46 | Notify | 03BRL-CAD:brlcad * 56609 brlcad/trunk/NEWS: richard improved the fast4-g importer in r56495 to skip blank lines |
| 23:36.50 | brlcad | starseeker: so they're not compatible ... or at least what I thought I was looking at the other day is different than what is in gdiam.cpp |
| 23:37.10 | brlcad | vec_point_2d is a class |
| 23:38.45 | brlcad | <PROTECTED> |
| 23:39.16 | brlcad | maybe even as simple as pointer value comparison |
