On Sun, Apr 15, 2012 at 9:12 PM, Goodwin Lawlor <span dir="ltr"><<a href="mailto:goodwin.lawlor.lists@gmail.com">goodwin.lawlor.lists@gmail.com</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5"><br><br><div class="gmail_quote">On Sat, Apr 14, 2012 at 8:01 PM, Goodwin Lawlor <span dir="ltr"><<a href="mailto:goodwin.lawlor.lists@gmail.com" target="_blank">goodwin.lawlor.lists@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br><br><div class="gmail_quote"><div><div>On Sat, Apr 14, 2012 at 5:32 PM, Marcus D. Hanwell <span dir="ltr"><<a href="mailto:marcus.hanwell@kitware.com" target="_blank">marcus.hanwell@kitware.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On Fri, Apr 13, 2012 at 7:31 PM, Goodwin Lawlor<br>
<div><div><<a href="mailto:goodwin.lawlor.lists@gmail.com" target="_blank">goodwin.lawlor.lists@gmail.com</a>> wrote:<br>
><br>
><br>
> On Fri, Apr 13, 2012 at 4:45 PM, Goodwin Lawlor<br>
> <<a href="mailto:goodwin.lawlor.lists@gmail.com" target="_blank">goodwin.lawlor.lists@gmail.com</a>> wrote:<br>
>><br>
>><br>
>><br>
>> On Fri, Apr 13, 2012 at 4:13 PM, Marcus D. Hanwell<br>
>> <<a href="mailto:marcus.hanwell@kitware.com" target="_blank">marcus.hanwell@kitware.com</a>> wrote:<br>
>>><br>
>>> On Fri, Apr 13, 2012 at 11:09 AM, Goodwin Lawlor<br>
>>> <<a href="mailto:goodwin.lawlor.lists@gmail.com" target="_blank">goodwin.lawlor.lists@gmail.com</a>> wrote:<br>
>>> ><br>
>>> ><br>
>>> > On Thu, Apr 12, 2012 at 6:35 PM, Marcus D. Hanwell<br>
>>> > <<a href="mailto:marcus.hanwell@kitware.com" target="_blank">marcus.hanwell@kitware.com</a>> wrote:<br>
>>> >><br>
>>> >> On Wed, Apr 11, 2012 at 2:09 PM, Marcus D. Hanwell<br>
>>> >> <<a href="mailto:marcus.hanwell@kitware.com" target="_blank">marcus.hanwell@kitware.com</a>> wrote:<br>
>>> >> > Hi,<br>
>>> >> ><br>
>>> >> > I have a Gerrit topic up for review,<br>
>>> >> ><br>
>>> >> > <a href="http://review.source.kitware.com/#/t/374/" target="_blank">http://review.source.kitware.com/#/t/374/</a><br>
>>> >> ><br>
>>> >> > This gets some of the rendering tests up and running again, but they<br>
>>> >> > hang until killed. Does anyone with deeper knowledge of Tcl and VTK<br>
>>> >> > have any idea what I might have missed. You can check out and build<br>
>>> >> > this topic, it depends on another topic that restores some Tcl code,<br>
>>> >> > and adjusts it for the new layout.<br>
>>> >> ><br>
>>> >> I have updated this topic, and have passing Tcl tests. The main point<br>
>>> >> I would love a little feedback on is why the additional require<br>
>>> >> package call was not necessary before modularization. I can confirm<br>
>>> >> that the image comparison is working, and I have to separate the tests<br>
>>> >> into two parts (Graphics and Rendering).<br>
>>> >><br>
>>> >> Marcus<br>
>>> ><br>
>>> ><br>
>>> > Hi Marcus,<br>
>>> ><br>
>>> > Pre-modularization, the vtk-tcl interpreter built with the vtk libs,<br>
>>> > initialized all the vtk commands on startup without a "package require<br>
>>> > vtk".<br>
>>> > That's the interpreter ctest uses to run rtImageTest.tcl.<br>
>>> ><br>
>>> > The question is, post-mod, what has changed that requires<br>
>>> > rtImageTest.tcl to<br>
>>> > have a "package require vtk".<br>
>>> ><br>
>>> > I'll have a look when I've got a new build.<br>
>>> ><br>
>>> Thanks for the explanation, I don't see where it was doing the package<br>
>>> require vtk before, and all the tests seem to have that as their first<br>
>>> line. For now we have merged the change, but I would welcome patches<br>
>>> if the old behavior is still desirable. We would also appreciate help<br>
>>> in restoring some of these tests if anyone still works with Tcl.<br>
>>><br>
>>> Thanks,<br>
>>><br>
>>> Marcus<br>
>><br>
>><br>
>> The "package require vtk" wasn't been done before for the interpreter<br>
>> built with the vtk libs - the vtk commands were been initialized by the<br>
>> interpreter itself (in c) - have a look at vtkTkAppInit.cxx (if you have<br>
>> time!)<br>
>><br>
>> If you were running vtk-tcl scripts with the interpreter above, "package<br>
>> require vtk" was (almost) redundant - but if you used your own/activestate's<br>
>> interpreter and built dynamic vtk libs with tcl wrappers, then you needed<br>
>> "package require vtk".<br>
>><br>
>> The merged change is fine - it's unlikely but someone may run the<br>
>> regression tests manually with a different interpreter and now they will<br>
>> work.<br>
>><br>
>> I'll build from master today and have a look through the tcl stuff over<br>
>> the weekend. We still use vtk-tcl in our lab...<br>
>><br>
>> Goodwin<br>
>><br>
><br>
> Just fyi, here's the diff that caused the above problems<br>
><br>
> <a href="http://vtk.org/gitweb?p=VTK.git;a=blobdiff;f=Wrapping/Tcl/vtkTkAppInit.cxx;h=8a6001416a69cf1ede821bce50c53f627768cd80;hp=857af94651b9f5366ede9ca9f62dad14c58d3229;hb=0c1471f1da0ee8d76e9db0d95a0c434f0b38a7f5;hpb=cdd4d6fdec8b4b2c143224ca1d94699c96a8b58a" target="_blank">http://vtk.org/gitweb?p=VTK.git;a=blobdiff;f=Wrapping/Tcl/vtkTkAppInit.cxx;h=8a6001416a69cf1ede821bce50c53f627768cd80;hp=857af94651b9f5366ede9ca9f62dad14c58d3229;hb=0c1471f1da0ee8d76e9db0d95a0c434f0b38a7f5;hpb=cdd4d6fdec8b4b2c143224ca1d94699c96a8b58a</a><br>
><br>
> The init code was chopped out of vtkTkAppInit.cxx since it reflected the old<br>
> structure, I guess.<br>
><br>
> I think it's better off this way, anyway.<br>
><br>
</div></div>I made that change, thanks for pointing out the change. If people want<br>
to bring it back I will know where to add the code back in now. I feel<br>
like it is better, but if it breaks a lot of old code I could see some<br>
argument for bringing it back.<br>
<span><font color="#888888"><br>
Marcus<br></font></span></blockquote><div><br></div></div></div><div>I discovered why the tcl interpreter was initialized like it was - it seems to be for the benefit of static builds, to force the linkage of static vtk libs. </div>
<div><br></div><div>Instead of re-instating the old code, it would be better to use the tcl function "Tcl_StaticPackage" to link in the vtk static libs. This would mean that the vtk commands only become available in a tcl interpreter after a "package require vtk" regardless if the build is shared or static... which is the way it should be.</div>
<div><br></div><div>BTW, the cmake variables:</div><div><br></div><div>VTK_USE_TK</div><div>VTK_TCL_TK_STATIC</div><div><br></div><div>were absent from the modular build. There were declared in the root CMakeLists.txt file.</div>
<div><br></div><div>VTK_TK_RESOURCES_DIR</div><div><br></div><div>was no longer being set.</div><div><br></div><div>The Tk include directories weren't being included...</div><div><br></div><div>I've got a static build with Tk working now, so I'll push something to gerrit to review.</div>
</div><br>
</blockquote></div><br></div></div><div>Here's my topic for review: <a href="http://review.source.kitware.com/#/t/415" target="_blank">http://review.source.kitware.com/#/t/415</a></div><div><br></div><div>A static modular vtk tcl build is working now.</div>
<div><br></div><div>Thanks,</div><div><br></div><div>Goodwin</div></blockquote><div><br></div><div>A topic for adding the Tcl tests for Common/* </div><div><br></div><div><a href="http://review.source.kitware.com/#/t/416">http://review.source.kitware.com/#/t/416</a></div>
<div><br></div><div>Running these tests, throws up some bugs in:</div><div><br></div><div>vtkAMREnzoReader</div><div><div>vtkAMREnzoParticlesReader</div><div>vtkStreamTracer</div><div>vtkProgrammableElectronicData</div><div>
vtkTestingObjectFactory </div></div><div><br></div><div>I'll submit a different topic for the fixes for these. Empty input bugs mainly...</div><div><br></div><div>Goodwin</div></div>