Bug 163178 - Terminal text locked at 0,0 when placed in a graphics view using a QGraphicsProxyWidget.
Summary: Terminal text locked at 0,0 when placed in a graphics view using a QGraphicsP...
Status: RESOLVED FIXED
Alias: None
Product: konsole
Classification: Applications
Component: general (show other bugs)
Version: unspecified
Platform: Compiled Sources Linux
: NOR normal
Target Milestone: ---
Assignee: Robert Knight
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-06-04 02:39 UTC by Jared Kells
Modified: 2009-05-05 17:21 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
Vt Test running in plasmoid. (361.40 KB, image/jpeg)
2008-06-10 13:29 UTC, Jared Kells
Details
Patch to fix 163178 (1.62 KB, patch)
2008-07-17 01:28 UTC, Jared Kells
Details
Konsole Plasmoid Test Case (3.56 KB, application/octet-stream)
2009-03-17 11:00 UTC, Jared Kells
Details
patch for the test-case (721 bytes, patch)
2009-03-21 19:25 UTC, Thomas Prochaska
Details
fixes test-case CMakeLists.txt (845 bytes, patch)
2009-05-03 22:03 UTC, Hatem Hassanein
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jared Kells 2008-06-04 02:39:50 UTC
Version:            (using Devel)
Installed from:    Compiled sources
OS:                Linux

I am trying to embed the Konsole KPart in a plasma plasmoid using a QGraphicsProxyWidget. Unfortunately the text area of the terminal sits locked in the top left corner of the screen no matter where I place the QGraphicsProxyWidget in the scene. 

Interestingly the scroll bar is drawn correctly so I suspected the konsole might be doing something a little funny.

I found three lines that move the konsole to the top left corner and reset the rotation. Removing them fixed my problem see attached patch for reference.

Robert Knight explains the reason for the code.
> They are there to support double width and double height lines.  Not a hugely
> important feature but it is there to make the "vttest" terminal test suite work 
> better.  The code can be rewritten so that it pushes a new transform matrix onto > he stack and pops it afterwards rather than resetting the painter's world
> transform back to the defaults after every line.  

PATCH:
Index: TerminalDisplay.cpp
===================================================================
--- TerminalDisplay.cpp	(revision 816076)
+++ TerminalDisplay.cpp	(working copy)
@@ -1390,8 +1390,8 @@
 		 //transformation has been applied to the painter.  this ensures that 
 		 //painting does actually start from textArea.topLeft() 
          //(instead of textArea.topLeft() * painter-scale)	
-		 QTransform inverted = paint.worldTransform().inverted();
-		 textArea.moveTopLeft( inverted.map(textArea.topLeft()) );
+		//QTransform inverted = paint.worldTransform().inverted();
+		//textArea.moveTopLeft( inverted.map(textArea.topLeft()) );
 		 
 		 //paint text fragment
          drawTextFragment(	paint,
@@ -1404,7 +1404,7 @@
 		 _fixedFont = save__fixedFont;
      
 		 //reset back to single-width, single-height _lines 
-		 paint.resetMatrix();
+		 //paint.resetMatrix();
 
 		 if (y < _lineProperties.size()-1)
 		 {
Comment 1 Jared Kells 2008-06-10 13:25:13 UTC
Hi here is a patch that fixes this bug whilst not regressing in the vttest suite. See attached screen shot.


Index: src/TerminalDisplay.cpp
===================================================================
--- src/TerminalDisplay.cpp     (revision 818969)
+++ src/TerminalDisplay.cpp     (working copy)
@@ -1372,15 +1372,21 @@
             _fixedFont = false;
          QString unistr(disstrU,p);

+                // Create a text scaling matrix for double width and double height lines.
+                QMatrix textScale;
                 if (y < _lineProperties.size())
                 {
                        if (_lineProperties[y] & LINE_DOUBLEWIDTH)
-                               paint.scale(2,1);
+                               textScale.scale(2,1);

                        if (_lineProperties[y] & LINE_DOUBLEHEIGHT)
-                               paint.scale(1,2);
+                               textScale.scale(1,2);
                 }

+                //Apply text scaling matrix.
+                paint.setWorldMatrix(textScale, true);
+
                 //calculate the area in which the text will be drawn
                 QRect textArea = QRect( _leftMargin+tLx+_fontWidth*x , _topMargin+tLy+_fontHeight*y , _fontWidth*le

@@ -1390,8 +1396,7 @@
                 //transformation has been applied to the painter.  this ensures that
                 //painting does actually start from textArea.topLeft()
          //(instead of textArea.topLeft() * painter-scale)
-                QTransform inverted = paint.worldTransform().inverted();
-                textArea.moveTopLeft( inverted.map(textArea.topLeft()) );
+                textArea.moveTopLeft( textScale.inverted().map(textArea.topLeft()) );

                 //paint text fragment
          drawTextFragment(     paint,
@@ -1404,7 +1409,7 @@
                 _fixedFont = save__fixedFont;

                 //reset back to single-width, single-height _lines
-                paint.resetMatrix();
+                paint.setWorldMatrix(textScale.inverted(), true);

                 if (y < _lineProperties.size()-1)
                 {
Comment 2 Jared Kells 2008-06-10 13:29:32 UTC
Created attachment 25237 [details]
Vt Test running in plasmoid.
Comment 3 Robert Knight 2008-07-16 21:19:52 UTC
Hi Jared

Can you attach the patch to the bug report using the "Create a New Attachment" link at the bottom of this report please.  Posting the patch inline as part of a comment messes up the formatting.

Adding patches as attachments also makes it possible to mark previous versions of a patch as obsolete.
Comment 4 Jared Kells 2008-07-17 01:28:29 UTC
Created attachment 26184 [details]
Patch to fix 163178

Hi Robert,

Patch is attached. The area of code I was changing used tab indentation. I used
the same in my patch so it matches, hope this is ok.

Kind Regards
Jared
Comment 5 Thomas Prochaska 2009-03-13 15:59:27 UTC
hi!

i just wanted to ask if this patch will be merged any time soon. the konsole-plasmoid is really cool but can't be used as long as this patch isn't applied.

is there a reason why this patch isn't applied? or maybe just forgotten.

regards
thomas prochaska
Comment 6 Kurt Hindenburg 2009-03-17 04:35:27 UTC
I have this up on reviewboard.kde.org.  Do you have some way for me to test it?

I'm not really up on plasma stuff.
Comment 7 Jared Kells 2009-03-17 11:00:06 UTC
Created attachment 32188 [details]
Konsole Plasmoid Test Case

This is a simple konsole plasmoid as a test case for this bug.
Comment 8 Kurt Hindenburg 2009-03-17 15:11:09 UTC
The test case won't compile on my 4.2 branch nor trunk.  Here's 4.2 error:

CMake Error at /mnt/kde/binaries/branches/KDE/4.2/share/apps/cmake/modules/FindPlasma.cmake:13 (message):                 
  FindPlasma.cmake is deprecated.  Now with KDE 4.2 Plasma is part of kdelibs                                             
  and automatically found using find_package(KDE4) instead.                                                               

  Replace the variables previously coming from FindPlasma.cmake as follows:

  PLASMA_OPENGL_FOUND -> KDE4_PLASMA_OPENGL_FOUND

  PLASMA_LIBS -> KDE4_PLASMA_LIBS

  PLASMA_INCLUDE_DIR -> KDE4_INCLUDE_DIR or KDE4_INCLUDES, should be already
  set anyway                                                                

  PLASMA_FOUND -> nothing, it's always there if KDE4, version 4.2 or newer
  has been found.                                                         

  If you see this error message in a module within KDE/, update it from svn,
  it has been fixed already.                                                

Call Stack (most recent call first):
  CMakeLists.txt:7 (find_package)
Comment 9 Thomas Prochaska 2009-03-21 19:25:48 UTC
Created attachment 32324 [details]
patch for the test-case

hi!

this patch should resolve the last reported cmake issue concerning the test-case (konsole plasmoid).

regards
thomas prochaska
Comment 10 Kurt Hindenburg 2009-03-25 05:35:30 UTC
Still can't compile the test case... no idea what's going on.  I have all of trunk compiled before.

Linking CXX shared module lib/plasma_applet_konsole.so                                   
CMakeFiles/plasma_applet_konsole.dir/konsoleapplet.o: In function `KonsoleApplet::qt_metacall(QMetaObject::Call, int, void**)':                                                   
/home/kdetrunk/konsole-0.1/konsoleapplet.moc:60: undefined reference to `Plasma::Applet::qt_metacall(QMetaObject::Call, int, void**)'
Comment 11 Hatem Hassanein 2009-05-03 22:03:21 UTC
Created attachment 33325 [details]
fixes test-case CMakeLists.txt

I managed to compile the test case with these changes (apply to Jared Kell's test case).
Comment 12 Kurt Hindenburg 2009-05-05 17:21:30 UTC
SVN commit 963912 by hindenburg:

Fix issue where terminal text is locked at 0,0.

BUG: 163178


 M  +12 -7     TerminalDisplay.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=963912