Bug 305323

Summary: Artistic text tool : Incoherent size and placement on canvas , unproductive and feels broken.
Product: krita Reporter: David REVOY <info>
Component: ToolsAssignee: Krita Bugs <krita-bugs-null>
Status: RESOLVED FIXED    
Severity: normal CC: beojan, halla, jpalaciosdev, nekomukuro, sven.langkamp
Priority: NOR    
Version: git master (please specify the git hash!)   
Target Milestone: ---   
Platform: Compiled Sources   
OS: Linux   
Latest Commit: Version Fixed In:
Attachments: illustration
Possible fix
Patch correction

Description David REVOY 2012-08-17 11:14:23 UTC
Same context as for https://bugs.kde.org/show_bug.cgi?id=305318 ; ( quote : From the conversation on the mailing about famous bug ; I must admit I totally forgive to enter bug for the text tool with Krita. But let's try to report  ; it would be great indeed if the text tool would work enought to enter a speechbuble or write a copyright signature.  )

Reproducible: Always

Steps to Reproduce:
1. Select in the Toolbar the Text Tool
2. Draw on the canvas a Square
3. Release button
4. A dummy text placeholder 'Artistic Text' appear ( almost wildly ) on the canvas
Actual Results:  
Observe where the dummy text placeholder 'Artistic Text' is placed, and the geometry of the size of the letter compared to the square you've just traced...
( Challenge : Try to draw a shape who makes the text  'Artistic Text'  normally shaped = prepare for high level skill game ) . It appears the tool is difficult to use, unproductive and feels broken. 

Expected Results:  
As a picture mockup tells more than 1000 words :
http://img827.imageshack.us/img827/3927/20120817screenshot001.jpg
( legend : In violet the shape traced on canvas by user , arrow show the movement )

rules I kept :
1.The heigh of the square traced by user could be the height of letters ( it sound it already to work currently )
2. The bottom line of the square could be the bottom line of the text, about vertical placement on the canvas.
3. The text could start at the left of the square ; easier for placement.
4. Any strech action should be defined by user *later* ( with deforming the bounding box) . So , I think the letter *shouldn't be deformed at creation* but keep proportion as they were created in the font file. Why ? Because creating weird proportion is more rare than wanting to trace good font as they were created. 

With this correction , Artistic Text Tool could easily does the job of creating a title and size it correctly visually by user, create a subtitle a bit less big , then with a little square on the bottom , create a signature.
Well , having all the text easy to trace and stored on a single vector layer like this sound like a dream , and I know Krita is really near to perform that.
Comment 1 Halla Rempt 2012-09-27 14:13:11 UTC
Lots of work to do... Thanks for the writeup.
Comment 2 Sven Langkamp 2012-11-06 22:35:20 UTC
Git commit 6fd50444dd18d3ccfc82d33280f6528b6fccf6d6 by Sven Langkamp.
Committed on 06/11/2012 at 23:34.
Pushed by langkamp into branch 'master'.

keep aspect ration when inserting artistic text with the text tool

M  +5    -0    krita/plugins/tools/tool_text/kis_tool_text.cc

http://commits.kde.org/calligra/6fd50444dd18d3ccfc82d33280f6528b6fccf6d6
Comment 3 beojan 2013-05-08 19:56:02 UTC
Created attachment 79784 [details]
illustration
Comment 4 beojan 2013-05-08 19:57:43 UTC
As can be seen in the above illustration attachment, both the position of the created artistic text, and the aspect ratio are still incorrect, with a recent git snapshot (version 2.8 pre alpha)
Comment 5 beojan 2013-05-10 19:16:49 UTC
Please reopen this bug. As per my comment above, the bug is still there in recent git snapshots.
Issues:
* Position of artistic text is shifted from rectangle drawn (grey rectangle indicates that drawn with artistic text tool)

* The Aspect ratio of the artistic text differs from that of the same font at the same size in Words (artistic text is wider than it should be)

* Font size displayed in the tool options docker is incorrect.
Comment 6 Halla Rempt 2013-05-17 09:14:12 UTC
Okay, let's reinvestigate further. Thanks for notifying us!
Comment 7 Annarosa Agarossi 2013-08-10 11:17:02 UTC
Created attachment 81628 [details]
Possible fix

Hi, 
I think I have found a fix for this bug as I was messing with the code to fix a (not yet reported as far as I see) bug that caused erratic multiline text shape placement in 2.7.1 (and was making my usual workflow like hell).
I attached a patch, which I made for the 2.7 branch that fixed both issues for me.
The cause of the weird size issue of the artistic text shape seems to be that the previously committed code that keeps aspect ratio was actually executed before the shape size was set. 
Also, the rect variable was used once in place of the r one, which caused the other bug with text placement I was originally trying to fix.

I hope this is useful.
Comment 8 Annarosa Agarossi 2013-08-10 11:21:21 UTC
(In reply to comment #7)
> Created attachment 81628 [details]
> Possible fix
> 
> Hi, 
> I think I have found a fix for this bug as I was messing with the code to
> fix a (not yet reported as far as I see) bug that caused erratic multiline
> text shape placement in 2.7.1 (and was making my usual workflow like hell).
> I attached a patch, which I made for the 2.7 branch that fixed both issues
> for me.
> The cause of the weird size issue of the artistic text shape seems to be
> that the previously committed code that keeps aspect ratio was actually
> executed before the shape size was set. 
> Also, the rect variable was used once in place of the r one, which caused
> the other bug with text placement I was originally trying to fix.
> 
> I hope this is useful.

Actually, I just noticed it just fixes this partially, as the positioning of the artistic text is still skewed, although the aspect ratio seems okay,
Comment 9 Halla Rempt 2013-08-10 12:37:29 UTC
Thanks! I'll test the patch :-)
Comment 10 Annarosa Agarossi 2013-08-10 14:36:16 UTC
Created attachment 81630 [details]
Patch correction

I think I found my mistake with the shape position, I inadvertently swapped the setPosition() and setSize() instructions. Somehow, this affected the artistic text shape position but not the multiline.
I attached a corrected patch that should fix it.
Comment 11 Halla Rempt 2013-09-26 19:32:39 UTC
Oops... August was like crazy for me, and I didn't even see there was a patch. I'll try to check this weekend!
Comment 12 Annarosa Agarossi 2013-09-27 12:06:51 UTC
After I have re-read better the OP message, I think I hadn't understood correctly about the artistic text shape sizing. Currently, with my patch, it fits to the box you draw, but if I have understood it correctly, it was meant to keep the font aspect ratio, right?
Since the function expects you to draw a rectangle, maybe it should keep the shortest side of the rectangle as size reference or something...?

Anyway with my last patch the text doesn't just jump around randomly and is sort-of usable for the time being. I'd like to tackle this issue, but I'd need to study better how the shape sizing works, and I'm currently pretty busy. If no-one can fix this in the meanwhile, I will try to get back at it when I have some vacation...
Comment 13 Sven Langkamp 2013-09-27 12:31:13 UTC
What your patch did was to revert it back to the original behavior that we had previously. It breaks the stuff that keeps the aspect ratio.

What's supposed to happen is that the shape keeps the height, but the width is adjusted to fit the text.
Comment 14 Annarosa Agarossi 2013-09-27 12:40:47 UTC
(In reply to comment #13)
> What your patch did was to revert it back to the original behavior that we
> had previously. It breaks the stuff that keeps the aspect ratio.
> 
> What's supposed to happen is that the shape keeps the height, but the width
> is adjusted to fit the text.

Yes, right, that's what I thought too. 
So, as far as I remember, the original commit kept aspect ratio but ended up producing huge text shapes that had little to no relation to the rectangle shape. Plus the positioning problem (which is the only thing that my patch actually fixes, I guess)

I hope to get some free time soon... I'd gladly try to tweak the function again so that the shape size fits the rectangle height alone.
Comment 15 Sven Langkamp 2013-09-27 12:42:55 UTC
Just saw that the patch actually includes the rigth fix. It was the rect -> r change :)

diff --git a/krita/plugins/tools/tool_text/kis_tool_text.cc b/krita/plugins/tools/tool_text/kis_tool_text.cc
index 5f2d410..e695330 100644
--- a/krita/plugins/tools/tool_text/kis_tool_text.cc
+++ b/krita/plugins/tools/tool_text/kis_tool_text.cc
@@ -54,7 +54,7 @@ void KisToolText::finishRect(const QRectF &rect)
     KoShapeFactoryBase* textFactory = KoShapeRegistry::instance()->value(shapeString);
     if (textFactory) {
         KoShape* shape = textFactory->createDefaultShape(canvas()->shapeController()->resourceManager());
-        shape->setPosition(rect.topLeft());
+        shape->setPosition(r.topLeft());
         // If the shape is an artistic shape we keep the aspect ratio so the text isn't stretched
         if (shapeString == "ArtisticText") {
             qreal ratio = rect.height()/shape->size().height();
Comment 16 Sven Langkamp 2013-09-27 12:44:53 UTC
Git commit f10371e688956a90d6d415270f6ab043a7f82f03 by Sven Langkamp.
Committed on 27/09/2013 at 12:43.
Pushed by langkamp into branch 'master'.

fix text shape position when inserted by the text tool
Based on patch by Annarosa Agarossi

M  +1    -1    krita/plugins/tools/tool_text/kis_tool_text.cc

http://commits.kde.org/calligra/f10371e688956a90d6d415270f6ab043a7f82f03
Comment 17 Sven Langkamp 2013-09-29 01:41:12 UTC
The text font size problem isn't in Krita, but rather in the shape itself. Font size and shape transformation are not connected. So the font is really 20, but it transformed to x times the size.
Comment 18 Juan Palacios 2014-02-01 15:29:40 UTC
Git commit 9a98faaa560bd70ece601d451e1e0f908a802b5b by Juan Palacios.
Committed on 01/02/2014 at 14:44.
Pushed by jpalacios into branch 'master'.

Fixed aspect ratio computation of artistic text shape

Aspect ratio was not being computed correctly. Now artistic shape
maintain the aspect ratio of the box used to define the text area,
keeping the defined height and extending the width to accommodate the
predefined text.

M  +2    -2    krita/plugins/tools/tool_text/kis_tool_text.cc

http://commits.kde.org/calligra/9a98faaa560bd70ece601d451e1e0f908a802b5b
Comment 19 Juan Palacios 2014-02-01 15:31:49 UTC
Git commit 44b7ca2cf517c08665d51d0efc42a66c9da53e69 by Juan Palacios.
Committed on 01/02/2014 at 14:44.
Pushed by jpalacios into branch 'calligra/2.8'.

Fixed aspect ratio computation of artistic text shape

Aspect ratio was not being computed correctly. Now artistic shape
maintain the aspect ratio of the box used to define the text area,
keeping the defined height and extending the width to accommodate the
predefined text.

M  +2    -2    krita/plugins/tools/tool_text/kis_tool_text.cc

http://commits.kde.org/calligra/44b7ca2cf517c08665d51d0efc42a66c9da53e69
Comment 20 Juan Palacios 2014-02-01 15:40:11 UTC
Just fixed the computation of aspect ratio. Now, the tool must work in the same way that David described above.
If all is fixed now, please feel free to close this bug.