DO NOT REPLY [Bug 39395] - PATCH: Placeholder shape and other improvements

2006-05-10 Thread bugzilla 
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=39395


[EMAIL PROTECTED] changed:

   What|Removed |Added

 Status|NEEDINFO|RESOLVED
 Resolution||FIXED




--- Additional Comments From [EMAIL PROTECTED]  2006-05-10 13:24 ---
(In reply to comment #4)
> >> Do you think have a nice helper for RGB for the text colour (and presumably
also
> >> background colour, shape colour etc), splitting into the tripplets, or
should we
> >> just stick with using the java.awt one?
> 
> End-user should work with  java.awt.Color. For internal purposes we
> can use either approach, but what we have now is OK to me.
> 
> If you are going to work with colors in PowerPoint, be aware it is not always
> logical:
> 
> - RGB values are swapped, i.e. it is 'BGR' instead of 'RGB'
> - alpha component is 254 for rich text runs and 0 for shape color.
> 
> There may be more 'features' we don't know about .

Hmm. Perhaps we will need our own wrapper longer term then, to handle these fun
features!

> >> Looking at TextBox, I'm not happy with how much of RichTextRun it seems to 
> >> be
> >> duplicating. Can we just provide a RichTextRun over the top of it?
Otherwise, we
> >> should refactor both of them to remove the duplication.
> 
> This is only temporary architecture. I have plans how to refactor it,
> just need time to put things in order in my head and start
> implementing it.

OK, sure. We should probably warn people that the API is subject to a lot of
change on it though!

> >> Also, why did you change TextCharsAtom to no longer use the common 
> >> StringUtil
> >> stuff? I didn't commit that, I didn't see the point!
> 
> Ops. My bad, sorry.
> This is experimental code and it came to the patch by mistake.
> 
> I studied why TextBox didn't display extended ASCII wingdings and
> decided to use TextCharsAtom by default, not TextBytesAtom when a new
> TextBox is created. Unfortunately I couldn't do it with current TextCharsAtom.

I've fixed StringUtils so it now works, and added a test for the TextCharsAtom
constructor stuff

> And what were your reasons to use StringUtil instead of
> constructing strings from byte array? I prefer to keep things simple.

I prefer not to duplicate code, when we already have handy util methods :)

Nick

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug, or are watching the assignee.

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
Mailing List:http://jakarta.apache.org/site/mail2.html#poi
The Apache Jakarta POI Project: http://jakarta.apache.org/poi/

DO NOT REPLY [Bug 39395] - PATCH: Placeholder shape and other improvements

2006-05-10 Thread bugzilla 
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=39395





--- Additional Comments From [EMAIL PROTECTED]  2006-05-10 07:30 ---


>> Do you think have a nice helper for RGB for the text colour (and presumably 
>> also
>> background colour, shape colour etc), splitting into the tripplets, or 
>> should we
>> just stick with using the java.awt one?

End-user should work with  java.awt.Color. For internal purposes we
can use either approach, but what we have now is OK to me.

If you are going to work with colors in PowerPoint, be aware it is not always
logical:

- RGB values are swapped, i.e. it is 'BGR' instead of 'RGB'
- alpha component is 254 for rich text runs and 0 for shape color.

There may be more 'features' we don't know about .

>> Looking at TextBox, I'm not happy with how much of RichTextRun it seems to be
>> duplicating. Can we just provide a RichTextRun over the top of it? 
>> Otherwise, we
>> should refactor both of them to remove the duplication.

This is only temporary architecture. I have plans how to refactor it,
just need time to put things in order in my head and start
implementing it.


>> Also, why did you change TextCharsAtom to no longer use the common StringUtil
>> stuff? I didn't commit that, I didn't see the point!

Ops. My bad, sorry.
This is experimental code and it came to the patch by mistake.

I studied why TextBox didn't display extended ASCII wingdings and
decided to use TextCharsAtom by default, not TextBytesAtom when a new
TextBox is created. Unfortunately I couldn't do it with current TextCharsAtom.

The following simple code results in error:

TextCharsAtom tca = new TextCharsAtom();
String txt = tca.getText();

ava.lang.ArrayIndexOutOfBoundsException: Illegal offset
at org.apache.poi.util.StringUtil.getFromUnicodeLE(StringUtil.java:67)
at org.apache.poi.util.StringUtil.getFromUnicodeLE(StringUtil.java:91)
at 
org.apache.poi.hslf.record.TextCharsAtom.getText(TextCharsAtom.java:44)

I looked at TextCharsAtom and simplified it a bit: construct UTF-16LE
string directly from a byte array. With this simplification it worked
fine.

And what were your reasons to use StringUtil instead of
constructing strings from byte array? I prefer to keep things simple.

Of course I would have discussed this change with you if I meant to commit it.

Regards, Yegor


-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug, or are watching the assignee.

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
Mailing List:http://jakarta.apache.org/site/mail2.html#poi
The Apache Jakarta POI Project: http://jakarta.apache.org/poi/

DO NOT REPLY [Bug 39395] - PATCH: Placeholder shape and other improvements

2006-05-08 Thread bugzilla 
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=39395


[EMAIL PROTECTED] changed:

   What|Removed |Added

 Status|NEW |NEEDINFO




--- Additional Comments From [EMAIL PROTECTED]  2006-05-08 16:42 ---
Cheers for this, I've applied it. Couple of things though:

Do you think have a nice helper for RGB for the text colour (and presumably also
background colour, shape colour etc), splitting into the tripplets, or should we
just stick with using the java.awt one?

Looking at TextBox, I'm not happy with how much of RichTextRun it seems to be
duplicating. Can we just provide a RichTextRun over the top of it? Otherwise, we
should refactor both of them to remove the duplication.

Also, why did you change TextCharsAtom to no longer use the common StringUtil
stuff? I didn't commit that, I didn't see the point!

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug, or are watching the assignee.

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
Mailing List:http://jakarta.apache.org/site/mail2.html#poi
The Apache Jakarta POI Project: http://jakarta.apache.org/poi/

DO NOT REPLY [Bug 39395] - PATCH: Placeholder shape and other improvements

2006-04-24 Thread bugzilla 
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=39395





--- Additional Comments From [EMAIL PROTECTED]  2006-04-24 14:45 ---
Created an attachment (id=18168)
 --> (http://issues.apache.org/bugzilla/attachment.cgi?id=18168&action=view)
patch with the changes


-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug, or are watching the assignee.

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
Mailing List:http://jakarta.apache.org/site/mail2.html#poi
The Apache Jakarta POI Project: http://jakarta.apache.org/poi/

DO NOT REPLY [Bug 39395] - PATCH: Placeholder shape and other improvements

2006-04-24 Thread bugzilla 
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=39395





--- Additional Comments From [EMAIL PROTECTED]  2006-04-24 14:45 ---
Created an attachment (id=18167)
 --> (http://issues.apache.org/bugzilla/attachment.cgi?id=18167&action=view)
source code


-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug, or are watching the assignee.

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
Mailing List:http://jakarta.apache.org/site/mail2.html#poi
The Apache Jakarta POI Project: http://jakarta.apache.org/poi/