[Bf-committers] Textobject: Patch is here

[Alexander Ewering]

On Fri, 17 Jun 2005, Ton Roosendaal wrote:

> Hi,
>
> OK. This ain't too bad at all, so for me you can go ahead!

Will do :) Glad you like it.

> Couple of remarks though;
>
> 1) Reading the diff:
>
> - in displist.c, remove the part with "tijd= clock". You can see in the diff 
> that was already removed by me, it causes cursor flicker on edit in 'solid' 
> drawmode.

Yes, I was a bit confused when I did those merges, and wasn't sure anymore if
it had been there or not.

> - the CU_FAST implementation is indeed hackish (the trick for filled fonts 
> right?)

Yes, it is. I've searched for the exception NOT to draw OB_FONTs filled during
editmode for 30 minutes or so, but haven't found it. Strange. All other
curve types are filled.

Will look, I indeed detest this hack, too.

> - split_font() uses OBACT and BASACT in weird ways... (dont do OBACT->)

Will check. I think OBACT or BASACT were macros which expand to
(BASACT ? blah) or something, which would indeed be bad in that
case.

> - the lorem.c has our (c) license... can't be OK

Should I remove the complete (c) part? I guess I can also cut that file
a bit... nobody will ever need that much Lorem Ipsum (it's indeed the
complete original text I think) :)

> - load_3d_text(), is this crossplatform? I never use fopen... forgot.

Yes, fopen is ANSI C. If not, we will notice soon enough ;-) And I'll
be there to fix.

> - a couple of more comments in code would be appreciated, like in 
> text_to_curve(), how this math works. Comments could help following your 
> thought process!

I understand... the problem is that sometimes, the math involved in that
part is so weird and goes around so many corners (fsize or not? Is the
offset already in? Or not? Is the offset already scaled by fsize? etc.)
that I ended up just trying all possible combinations until it was
aligned OK. Faster than thinking (at least for my slow thinking :)

> 2) Compiling
>
> - font.c, warnings "use unitialized vars"

I noticed, easy fix.

> - editfont.c, warnings "limited data range" (smells bad :)

That's because cu->len is a (signed) short, and the #define MAXTEXTLEN
was set to 50000 (by me). Setting it at 32766 will get rid of that
warning.

> - editobject.c, pointer/int warning

Yes, I don't understand how adduplicate() was supposed to work! You pass
a float* to it, but its value is never actually used, it's only checked
if it is 0 (not NULL) or not. It's used as a flag rather, to detect if
it is interactive duplication or not. It should probably become an int.

> 3) Using
>
> - crash with Add Text, press "Lorem" button

Can't reproduce. Will check the relevant code.

> Further played a little while with it, seems all to work nicely!
> Some ideas to check;
>
> - Panel composition (Matt?)
> - disable 3d cursor in text editmode, and use LMB to position text cursor

Sounds like a very good idea. Could someone else do this? I'm not the
OpenGL/Matrix/Math guy ;-)

> - interactive dragging of the 'text boxes'

Yes, I already talked to theeth about that. Really important issue, I would
like to see it. Maybe CTRL-G to grab, and CTRL-S to scale. (G and S are
obviously taken :P)

> - Maybe some default text box size to start with (or make the text boxes be a 
> setting on/off, with an initialize when you turn it on)

I've made the code so that a textbox size of 0 in either direction disables
any action (wrapping/flowing into next box) in that direction, so that old
files load OK.

Concerning old files: We need to up the version number to 238, so I can put
the compatibility code in readfile.c's do_versions (cu->wordspace needs
to be initialized to 1.0, otherwise people will get weird whitespace).
Alternatively, it could of course check for cu->wordspace == 0.0 (nonsensical
anyway), and just set it to 1.0 then.

> And yes, someone should fix the displist stuff. Come on, not too hard! Nice 
> job for someone here the next 2 weeks!

:-)

| alexander ewering              instinctive mediaworks
| ae[@]instinctive[.]de   http://www[.]instinctive[.]de