[vtkusers] Experimental enhanced wrappers for VTK
David Gobbi
david.gobbi at gmail.com
Mon Jun 7 10:38:56 EDT 2010
Please wait a bit before trying the fix that I just suggested... I'm
pretty sure that I made a mistake on it. Later this morning I'll be
able to take a closer look.
David
On Mon, Jun 7, 2010 at 8:35 AM, David Gobbi <david.gobbi at gmail.com> wrote:
> Hi Keith,
>
> That code is supposed to pull out the text from function pointer
> definitions like the following, in Graphics/vtkWarpScalar.h:88:
>
> double *(vtkWarpScalar::*PointNormal)(vtkIdType id, vtkDataArray *normals);
>
> So the loop should use i and j to mark the start and end of
> "vtkWarpScalar::". But I see the problem now: it fails in the case of
> a simple "(*" because there is an extra "i++" that doesn't belong:
>
>
> "("[\t\n\r ]*([a-zA-Z_][a-zA-Z0-9_]*::)*"*" {
>
> int i = 1;
>
> int j;
>
> while (yytext[i]==' ' || yytext[i]=='\t' ||
>
> yytext[i]=='\r' || yytext[i]=='\n') { i++; }
>
> j = i; /* <- HERE IS WHERE THE i++ WAS REMOVED */
>
> while (yytext[j]!='*') { j++; }
>
> yylval.str = (char *)malloc(j-i+1);
>
> if (j > i) { strncpy(yylval.str, &yytext[i], j-i); }
>
> yylval.str[j-i] = '\0';
>
> return(LPAREN_POINTER);
>
> }
>
> Try that out and let me know if valgrind likes it.
>
> To give a bit of background, I added LPAREN_POINTER because I was
> having trouble parsing function pointer declarations in vtkParse.y.
> My attempts to parse them would cause shift/reduce errors, so I
> decided to do some of the parsing in vtkParse.l instead.
>
> David
>
>
> On Mon, Jun 7, 2010 at 7:48 AM, Keith Fieldhouse
> <keith.fieldhouse at kitware.com> wrote:
>> Hi David,
>> As part of the testing to get your wrapping stuff merged, we built on
>> Windows 64 bit and ran into some trouble.
>> The first was a crash during vtkWrapPython that was just an uninitialized
>> variable. The fix can be seen at
>> http://github.com/keith-fieldhouse/VTK/commit/c6d900
>> If you're interested.
>> The other is a little more troublesome and I'm hoping you can help me
>> understand it. I'm getting random failures during vtkWrapJava under Visual
>> Studio. The failures manifest themselves differently depending on how
>> things are run (things proceed normally under nmake, but crash under Visual
>> Studio for example) which leads to the suspicion of some sort of memory
>> error.
>> Valgrind under 64bit Linux provides a clue. One of the files that tends to
>> fail wrapping is Common/vtkMatrix4x4.h while Common/vtkMatrix3x3.h hasn't
>> exhibited a failure. Comparing the valgrind output for the two files
>> suggests the following.
>> In vtkParse.l after commit 047dc6ca the LPAREN_POINTER stanza looks like
>> this:
>>
>> "("[\t\n\r ]*([a-zA-Z_][a-zA-Z0-9_]*::)*"*" {
>> int i = 1;
>> int j;
>> while (yytext[i]==' ' || yytext[i]=='\t' ||
>> yytext[i]=='\r' || yytext[i]=='\n') { i++; }
>> i++; j = i;
>> while (yytext[j]!='*') { j++; }
>> yylval.str = (char *)malloc(j-i+1);
>> if (j > i) { strncpy(yylval.str, &yytext[i], j-i); }
>> yylval.str[j-i] = '\0';
>> return(LPAREN_POINTER);
>> }
>>
>> Valgrind emits a number of errors that suggest that in the
>> while(yytext[j]!='*'), A '*' is never found and j gets incremented beyond
>> the bounds of yytext and hijinx ensue. While I'm not certain that this is
>> causing the crashes I'm seeing, it is suggestive. My experience with Lex
>> dates back more years than I care to think about so I'm not entirely sure
>> why you're looking for another '*' here? Or maybe I'm just misreading the
>> code entirely, which is why I'm asking for your help. Does my analysis make
>> sense and is there an approach you can see that would eliminate the problem?
>> Thanks again for all of your help.
>> Keith
>>
>> On Fri, May 28, 2010 at 12:48 PM, David Gobbi <david.gobbi at gmail.com> wrote:
>>>
>>> Hi Marcus,
>>>
>>> If you can get it merged by next week, that would be great.
>>>
>>> From my own tests, the only methods that will still need BTX/ETX after
>>> the merge are methods that take pointers to special type, because the
>>> wrappers will think these are vtkObjectBase-types. I have a list of
>>> these methods that I can send along after the merge. It's easy to
>>> find these methods, because the Java wrappers fail to build when it
>>> encounters them.
>>>
>>> To solve this issue in the long term, I'm working on a tool that can
>>> make a text file that describes the VTK class hierarchy (both for
>>> vtkObjects and for special VTK types). Through the use of this file,
>>> when the wrappers encounter a parameter called "vtkSomething" they
>>> will know at compile-time whether vtkSomething is a vtkObject.
>>>
>>> David
>>>
>>>
>>> On Fri, May 28, 2010 at 10:09 AM, Marcus D. Hanwell
>>> <marcus.hanwell at kitware.com> wrote:
>>> > Hi David,
>>> > Thanks for all of the work you have put into this. David Cole is going
>>> > to
>>> > look over the topic branch, hopefully at some point today. If he doesn't
>>> > spot any major issues I would like to merge this into master early next
>>> > week.
>>> > I am certainly excited about the improvements you are bringing to the
>>> > wrapping in VTK.
>>> > Thanks,
>>> > Marcus
>>> > On Thu, May 27, 2010 at 4:52 PM, David Gobbi <david.gobbi at gmail.com>
>>> > wrote:
>>> >>
>>> >> I've pushed the last of the changes to my "wrapping" branch on github.
>>> >> It's ready for merge.
>>> >>
>>> >> David
>>> >>
>>> >>
>>> >> On Wed, May 26, 2010 at 4:49 PM, David Gobbi <david.gobbi at gmail.com>
>>> >> wrote:
>>> >> > In the new wrappers, there is already full support vtkStdString
>>> >> > support Tcl and Java, and I've even added string regression test for
>>> >> > Tcl.
>>> >> >
>>> >> > Unicode was easy to add to Python (a few hours work), and it should
>>> >> > be
>>> >> > similarly easy for someone to add it for Tcl and Java. There are
>>> >> > plenty of people who want it, I'm sure.
>>> >> >
>>> >> > Divergence is a concern, but at least all wrappers use the same
>>> >> > front-end, so transferring enhancements from one set of wrappers to
>>> >> > another is that much easier. And Brad King mentioned a wrapper
>>> >> > utility .cxx that all wrappers use... this would be great, because
>>> >> > right now there is a huge amount of code duplication.
>>> >> >
>>> >> > I'll switch over to the branch immediately, and in the future I will
>>> >> > make branches for any other projects that involve multiple classes.
>>> >> >
>>> >> > David
>>> >> >
>>> >> >
>>> >> > On Wed, May 26, 2010 at 3:58 PM, Marcus D. Hanwell
>>> >> > <marcus.hanwell at kitware.com> wrote:
>>> >> >> Hi David,
>>> >> >> On Wed, May 26, 2010 at 5:46 PM, David Gobbi <david.gobbi at gmail.com>
>>> >> >> wrote:
>>> >> >>>
>>> >> >>> Thanks for doing this! I was not looking forward to doing the
>>> >> >>> clean-up work myself... when I started it never occurred to me to
>>> >> >>> create a branch, my plan was to eventually re-base my changes on
>>> >> >>> the
>>> >> >>> VTK master.
>>> >> >>
>>> >> >> Topic branches are much easier for us to work with, and if you avoid
>>> >> >> merging
>>> >> >> into them/rebasing then the eventual merge into master becomes
>>> >> >> simpler.
>>> >> >> Now
>>> >> >> that we have it you could continue your work in that branch, or we
>>> >> >> could
>>> >> >> cherry pick the commits you make.
>>> >> >>>
>>> >> >>> There are just two minor changes that I can squeeze in over the
>>> >> >>> next
>>> >> >>> two days, and I will email you when they are done. After that, the
>>> >> >>> sooner that the branch can be merged into master, the better. In
>>> >> >>> order for the special-type wrapping to be fully realized, most (or
>>> >> >>> all) of the BTX's will have to be removed from the code. A change
>>> >> >>> that big will probably be easier to do in the master than to merge
>>> >> >>> in
>>> >> >>> from a branch.
>>> >> >>
>>> >> >> A global change like that is definitely something that would be
>>> >> >> easier
>>> >> >> done
>>> >> >> in master as you say. We can take care of that once it looks ready
>>> >> >> to
>>> >> >> go
>>> >> >> in.
>>> >> >>>
>>> >> >>> I've already tested VTK (with python, tcl, and java) without
>>> >> >>> BTX/ETX.
>>> >> >>> The Tcl and Java wrappers compiled and tested cleanly, and so did
>>> >> >>> Python after I did some small fixes to the way that it does special
>>> >> >>> type wrapping. It is these python fixes that I need to commit
>>> >> >>> before
>>> >> >>> the merge.
>>> >> >>>
>>> >> >> I think one of the remaining concerns is how much the wrapping might
>>> >> >> diverge, i.e. if Python supports vtkVariant, unicode, vtkStdString
>>> >> >> etc
>>> >> >> but
>>> >> >> other languages did not. I am certainly very pleased with the
>>> >> >> changes
>>> >> >> in
>>> >> >> Python, and can see how much it will improve the Python wrapping in
>>> >> >> charts
>>> >> >> and infovis.
>>> >> >>
>>> >> >>>
>>> >> >>> There are other changes that I'm planning for the Python wrappers
>>> >> >>> (enums, etc) but for them I'd prefer to wait until after the merge.
>>> >> >>> I'll put together a wiki page so that they can be reviewed.
>>> >> >>>
>>> >> >> You could publish them in a topic branch for us to review. The main
>>> >> >> caveats
>>> >> >> remain to avoid rebasing/merging from master. Once we are ready to
>>> >> >> merge
>>> >> >> into master we can take care of that. Thanks for putting this
>>> >> >> together,
>>> >> >> and
>>> >> >> to Keith for preparing this topic branch.
>>> >> >> I am very hopeful that we can merge these changes in soon. We have
>>> >> >> been
>>> >> >> working away behind the scenes to get this ready to be reviewed and
>>> >> >> merged
>>> >> >> in.
>>> >> >> Thanks,
>>> >> >> Marcus
>>> >> >>>
>>> >> >>> On Wed, May 26, 2010 at 2:39 PM, Keith Fieldhouse
>>> >> >>> <keith.fieldhouse at kitware.com> wrote:
>>> >> >>> > David
>>> >> >>> > I've been spending some time looking at your Wrapping work. It
>>> >> >>> > appears
>>> >> >>> > that
>>> >> >>> > your git tree was built with a mixture of rebasing and merging
>>> >> >>> > which
>>> >> >>> > resulted in a somewhat wonky commit history (duplicated commits
>>> >> >>> > interspersed
>>> >> >>> > with commits coming from VTK master).
>>> >> >>> > In order to get a clean topic branch that will be a little easier
>>> >> >>> > for
>>> >> >>> > folks
>>> >> >>> > here to look at (and to eventually merge with master) I spent a
>>> >> >>> > bit
>>> >> >>> > of
>>> >> >>> > time
>>> >> >>> > untangling your history (mostly by examining parentage and
>>> >> >>> > looking
>>> >> >>> > at
>>> >> >>> > commit
>>> >> >>> > subjects). In the end, by cherry picking the right commits in
>>> >> >>> > what
>>> >> >>> > I
>>> >> >>> > considered the "right" order I was able to build a clean topic
>>> >> >>> > branch
>>> >> >>> > with
>>> >> >>> > no conflicts. Once I had that, I rebased it against master (as
>>> >> >>> > of
>>> >> >>> > this
>>> >> >>> > afternoon sometime when I did it). The result can be found in
>>> >> >>> > git://github.com/keith-fieldhouse/VTK.git
>>> >> >>> > As the topic branch Gobbi/Wrapping.
>>> >> >>> > The result builds and the tests run so I believe that I've built
>>> >> >>> > a a
>>> >> >>> > representative branch from your work.
>>> >> >>> > If you plan on doing additional work on this, would it be
>>> >> >>> > possible
>>> >> >>> > for
>>> >> >>> > you
>>> >> >>> > to grab this topic branch and work on that (without rebasing or
>>> >> >>> > merging
>>> >> >>> > for
>>> >> >>> > now)? That way we'll have a clean topic branch for an
>>> >> >>> > eventual
>>> >> >>> > merge
>>> >> >>> > to
>>> >> >>> > "Next" when we switch to branchy development for VTK. Or if
>>> >> >>> > branchy
>>> >> >>> > development comes later, this clean topic branch will be easier
>>> >> >>> > to
>>> >> >>> > rebase as
>>> >> >>> > well.
>>> >> >>> > Thanks again for your work on this.
>>> >> >>> > Keith
>>> >> >>> > --
>>> >> >>> > Keith Fieldhouse
>>> >> >>> > R&D Engineer, Kitware Inc.
>>> >> >>> > (518) 836-2190
>>> >> >>> >
>>> >> >>> >
>>> >> >>> >
>>> >> >>> >
>>> >> >>
>>> >> >>
>>> >> >
>>> >
>>> >
>>
>>
>
More information about the vtkusers
mailing list