You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@poi.apache.org by bu...@apache.org on 2006/04/24 16:44:43 UTC

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

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=39395>.
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

           Summary: PATCH: Placeholder shape and other improvements
           Product: POI
           Version: unspecified
          Platform: Other
        OS/Version: other
            Status: NEW
          Severity: normal
          Priority: P2
         Component: HSLF
        AssignedTo: poi-dev@jakarta.apache.org
        ReportedBy: yegor@dinom.ru


(1) Obsolete classes which go away:
org.apache.poi.hslf.model.Ellipse.java
org.apache.poi.hslf.model.Rectangle.java

(2) TextBox didn't properly handle text in placeholders. 
Now the problem is fixed. 
Placeholder is a new subclass of TextBox which handles such text.
I also had to implement a couple of new records:

OEPlaceholderAtom.java
 - Describes a placeholder. 

OutlineTextRefAtom.java
 - If present it indicates that TextHeaderAtom, TextBytesAtom and StyleTextPropAtom
are stored in Document.SlideListWithText instead of the shape'S escher container.
The content is just a 0-based index of the [TextHeaderAtom, TextBytesAtom and
StyleTextPropAtom] block in SlideListWithText.

If you iterate over text shapes in a slide and collect them in a set 
it must be the same as returned by Slide.getTextRuns().
The test code is in TestShapes.testTextBoxSet() . It is a good sanity check
which revealed some bugs.

(4) Placeholder shape. It is a subclass of TextBox which stores text attributes
outside of it's escher container.

For now I can create the only type of placeholder: slide title.
Use case:

 Slide slide = ppt.createSlide(); //blank slide

 TextBox title = slide.addTitle(); //add title placeholder. 
 title.setText("Hello, World");

...

 If you edit this shape in powerpoint you can
  (a) see that it is a placeholder
  (b) see that it holds slide's title. Outline view shows the  correct title text .

I also added the sample code in how-to-shapes.xml

(5) Added getFontColor and setFontColor to RichTextRun. Now I think all
character format properties are covered.

Regards, Yegor Kozlov

-- 
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: poi-dev-unsubscribe@jakarta.apache.org
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

Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=39395>.
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 yegor@dinom.ru  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: poi-dev-unsubscribe@jakarta.apache.org
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

Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=39395>.
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 yegor@dinom.ru  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: poi-dev-unsubscribe@jakarta.apache.org
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

Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=39395>.
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 yegor@dinom.ru  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: poi-dev-unsubscribe@jakarta.apache.org
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

Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=39395>.
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


nick@torchbox.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |NEEDINFO




------- Additional Comments From nick@torchbox.com  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: poi-dev-unsubscribe@jakarta.apache.org
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

Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=39395>.
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


nick@torchbox.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEEDINFO                    |RESOLVED
         Resolution|                            |FIXED




------- Additional Comments From nick@torchbox.com  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: poi-dev-unsubscribe@jakarta.apache.org
Mailing List:    http://jakarta.apache.org/site/mail2.html#poi
The Apache Jakarta POI Project: http://jakarta.apache.org/poi/