Opened 12 years ago

Closed 9 years ago

#4885 closed patch (wontfix)

multi line BiDi support in OSD

Reported by: ido <ido_roseman@…> Owned by: danielk
Priority: minor Milestone: 0.24
Component: MythTV - User Interface Library Version: head
Severity: medium Keywords: hebrew
Cc: stuartm Ticket locked: no

Description

short description : current hebrew/arabic support on OSD was not showing multyline text (such as program details) correctly. administrative : this is an hybrid of the work of sagi (in patch #4252 ) and thelinuxer (patch #4191 ) + some bug fixes. i've also changed required fribidi version from 0.19 to 0.10, as fribidi2 is still unstable and version 10.x is widely available. long description : if you take a long hebrew string, prepare it for rendering and only then split it into lines, you get the text going from botton to top (which is wrong) since the splitter see's the string revered. having the log2vis in TTFFont class is more "correct" in a systematic view and indid avoids this problem.

Attachments (12)

bidiosd.patch (8.9 KB) - added by ido <ido_roseman@…> 12 years ago.
4885-v1.patch (13.7 KB) - added by danielk 11 years ago.
Updated to apply to trunk + memleak and other fixes.
4885-v2.patch (21.1 KB) - added by danielk 11 years ago.
oops, missed some files and this is actually based on #6004
patch.output (1.3 KB) - added by Tony Arie Kolev <kolevtony@…> 11 years ago.
I have tested v2 using last release 19440 and inspected patch rejects
4885-v3.patch (21.0 KB) - added by Tony Arie Kolev <kolevtony@…> 11 years ago.
Rejects corrected, v3 created, error in compilation #include <QString> not found, changed to #include <qstring.h>, now error in compilation: /usr/lib/qt-3.3/include/qstring.h:856: error: ‘void QString::detach()’ is private
Archive.tar.gz (239.5 KB) - added by shed2k@… 11 years ago.
this file contains screen shots of my corrections to the OSD bidi support. It is based on Ido's original patch.
xml.rtl.patch (2.5 KB) - added by Tony Arie Kolev 11 years ago.
Additional RTL fixes - texts in main menu and submenus are falling out of right edge of the buttons in various themes, Daniel, please add it in v2 patch and see also my comment about compilation problem
osd_rtl.patch (11.9 KB) - added by Ran Nachmany 11 years ago.
This patch is built on top of Ido's bidi fix. It fixes the OSD and menu behavior, as well as some right alignment. The results of this patch can be seen in previously attached screenshots.
osd.xml.4.patch (4.1 KB) - added by Tony Arie Kolev <kolevtony@…> 11 years ago.
More fixes of rtl texts falling out of right side of field (for Daniel)
osd.rtl.v5.patch (4.7 KB) - added by Tony Arie Kolev <kolevtony@…> 11 years ago.
Tested and reccommended
4885-v5.patch (24.6 KB) - added by Tony Arie Kolev <kolevtony@…> 11 years ago.
Finally Tested with development version
4885-v5a.patch (24.6 KB) - added by Tony Arie Kolev <kolevtony@…> 11 years ago.
Corrected our -> your in GPL statement

Download all attachments as: .zip

Change History (43)

Changed 12 years ago by ido <ido_roseman@…>

Attachment: bidiosd.patch added

comment:1 Changed 12 years ago by Isaac Richards

Milestone: 0.210.22

comment:2 Changed 12 years ago by danielk

Owner: changed from Isaac Richards to danielk
Status: newassigned

comment:3 Changed 11 years ago by anonymous

note: it also fixes RTL iaauea on the on-screen menu (which previous patches did not)

comment:4 Changed 11 years ago by kolevtony@…

Hello, I just uploaded patch for OSD: http://svn.mythtv.org/trac/ticket/6004 this one fixes left-to-right displays, bottom-to-top displays and left alinment. Thanks for you patch I started to learn the insides of RTL manipulations in MythTV and in my opinion it is a apropriate to inform You how I think the thinks must be done. I understatnd that a lot of work will be spend to make it really full RTL compliant, but for now it is my best. Have a nice day!

Changed 11 years ago by danielk

Attachment: 4885-v1.patch added

Updated to apply to trunk + memleak and other fixes.

Changed 11 years ago by danielk

Attachment: 4885-v2.patch added

oops, missed some files and this is actually based on #6004

comment:5 Changed 11 years ago by danielk

Status: assignedinfoneeded

Ido & Tony, it appears you are trying to solve the same problem. I've fixed some memleaks and other problems in Tony's patch and attached the result here. I have not verified that this works.

Tony, do we really need to create a new RTL instance for each OSDListBtnTypeItem? Could not one instance be used for the entire list, since only one list item is drawn at a time?

Ido, can you confirm this patch solves all the problems you were experiencing?

Changed 11 years ago by Tony Arie Kolev <kolevtony@…>

Attachment: patch.output added

I have tested v2 using last release 19440 and inspected patch rejects

Changed 11 years ago by Tony Arie Kolev <kolevtony@…>

Attachment: 4885-v3.patch added

Rejects corrected, v3 created, error in compilation #include <QString> not found, changed to #include <qstring.h>, now error in compilation: /usr/lib/qt-3.3/include/qstring.h:856: error: ‘void QString::detach()’ is private

comment:6 Changed 11 years ago by Tony Arie Kolev <kolevtony@…>

Hello, Daniel, I have recently checked out ver.19440 from: svn co http://svn.mythtv.org/svn/branches/release-0-21-fixes/ I got 4885-v2.patch and tried to compile. I agree, memleaks fixes are OK, but some rejects was expected when patch applied, after fixing it (see 4885-v3.patch) and compiling , there was error: /usr/lib/qt-3.3/include/qstring.h: In member function ‘void OSDTypeText::SetText?(const QString&)’: /usr/lib/qt-3.3/include/qstring.h:856: error: ‘void QString::detach()’ is private osdtypes.cpp:788: error: within this context I have NOT succeed to build from sources. Please, review it again.

comment:7 Changed 11 years ago by danielk

Status: infoneededassigned

Tony, did you see my comment on the dev list? My patch is for svn head where it will be applied, not to the 0.21-fixes branch.

comment:8 Changed 11 years ago by danielk

Status: assignedinfoneeded

comment:9 Changed 11 years ago by anonymous

Daniel, tony's description talks about the same issues I raised (and some more). I'm brushing off the dust from my development environment, but still having problems to compile trunk, so it will take me couple of days to verify it actually works. as mentioned before, I have some notes on implementation, but I would not argue with results :-) . I'll keep you posted.

Changed 11 years ago by shed2k@…

Attachment: Archive.tar.gz added

this file contains screen shots of my corrections to the OSD bidi support. It is based on Ido's original patch.

comment:10 Changed 11 years ago by Ran Nachmany

Ido, Daniel, Tony,

I have used Ido's original patch and added correction to the same problems Tony is talking about and some more inclding:

  1. OSD behavior will be from right to left if "he" lang is selected (e.g. the menu will be displayed in the upper right side of the screen, the sub-trees will be draws left to it).
  2. Recorded programs screen: Added support for <rtl></rtl> attribute in the theme file. If the windows is defined as <rtl> the entire behavior will change. The program list will be drawn at the right side of the screen, the recorded episodes will be draws in the left side of the screen, according to the theme file. The key handling will be changed as well (left == ok, right == back).

my current development environment is in the middle of other fixes i am doing (adding rtl to EPG, fixing Hebrew support in rip CD etc.). I will isolate the changes i made for this specific patch and will upload it shortly.

The results of my fixes can be seen in the attached screen shots.

comment:11 Changed 11 years ago by Tony Arie Kolev

Daniel, I got the development svn rel. 19465 and applied the v2 patch. configure script needed to install qt4 (I installed qt-devel-4.3.4-11 package). Now we have problem in compilation: rtl.cpp:80: error: 'QCString' was not declared in this scope.

Changed 11 years ago by Tony Arie Kolev

Attachment: xml.rtl.patch added

Additional RTL fixes - texts in main menu and submenus are falling out of right edge of the buttons in various themes, Daniel, please add it in v2 patch and see also my comment about compilation problem

Changed 11 years ago by Ran Nachmany

Attachment: osd_rtl.patch added

This patch is built on top of Ido's bidi fix. It fixes the OSD and menu behavior, as well as some right alignment. The results of this patch can be seen in previously attached screenshots.

Changed 11 years ago by Tony Arie Kolev <kolevtony@…>

Attachment: osd.xml.4.patch added

More fixes of rtl texts falling out of right side of field (for Daniel)

comment:12 Changed 11 years ago by Tony Arie Kolev <kolevtony@…>

Daniel, I have tested parts of Ran Nachmany's patch (that patches osdlistbtntype.h & osdlistbtntype.cpp) and integrated it successfully in my patches (I am attaching it how it looks in stable release branch after my other patches have been applied - see osd.rtl.v5.patch) - I like to recommend it to You to include it in You work. Unfortunately , I did not actually tested You patch, I just succeeded to compile mythtv, after changing QCString to Q3CString in rtl.cpp, but I am not able to run it, because of incompatibility with data base ( I have only one, stable, running system and I do not want to play with it, there I am using a stable release branch). What stable release will include rtl patches?

Changed 11 years ago by Tony Arie Kolev <kolevtony@…>

Attachment: osd.rtl.v5.patch added

Tested and reccommended

comment:13 Changed 11 years ago by Tony Arie Kolev <kolevtony@…>

Just forgot,for osd.rtl.v5.patch to be complete , a mythtv/themes/default/lb-ltarrow.png must be created (a mirror of lb-arrow.png)

comment:14 Changed 11 years ago by danielk

Patches against 0.21-fixes will not be applied. I need someone to test 4885-v3.patch against head and make any changes necessary to that patch.

comment:15 in reply to:  14 Changed 11 years ago by Tony Arie Kolev <kolevtony@…>

Replying to danielk:

Patches against 0.21-fixes will not be applied. I need someone to test 4885-v3.patch against head and make any changes necessary to that patch.

OK, I will prepare a dedicated system for testing, I am buying a new PVR-150, it will take 1-2 weeks.

Changed 11 years ago by Tony Arie Kolev <kolevtony@…>

Attachment: 4885-v5.patch added

Finally Tested with development version

comment:16 Changed 11 years ago by Tony Arie Kolev <kolevtony@…>

Daniel , it took a long time, but finally I prepared environment & hardware for a testing purposes and downloaded a recent development version and after some adjustments successfully compiled and tested the patch (attached here - see osd.rtl.v5.patch). Additionally I have included a Ran Nachmany's code integrated in our patch (I recommended it 2 months ago). All the code is doing correctly its job and I confirm that it is OK. It is recommended to include it as soon as possible in mainstream code. Thanks.

Not related to our patch , I have to mention - in main menu and also in play recordings menus, the RTL texts are left aligned that is Wrong. In stable release RTL texts in main menu was right aligned and it is Correct. Again - it is not a part of our patch, but needs attention.

Tony Kolev, Linux Professional Institute Certified (LPIC-2)

comment:17 Changed 11 years ago by danielk

...Distributed as part of MythTV under the GPLv2, or at <b>our</b> option a later...

Highlighting mine.. well that makes this a pretty useless grant :) it should be "your", please correct.

comment:18 Changed 11 years ago by danielk

Resolution: invalid
Status: infoneededclosed

no response to info needed

comment:19 Changed 11 years ago by Tony Arie Kolev <kolevtony@…>

What that means, Daniel , after months of work, and waiting for testing feedback, You closed the ticked to invalid?

Changed 11 years ago by Tony Arie Kolev <kolevtony@…>

Attachment: 4885-v5a.patch added

Corrected our -> your in GPL statement

comment:20 Changed 11 years ago by Tony Arie Kolev <kolevtony@…>

Hi, after Michael T. Dean explained me clearly, I corrected our -> your in GPL statement in rtl.cpp. Please proceed the patch as soon as possible.

comment:21 Changed 11 years ago by danielk

Resolution: invalid
Status: closednew

comment:22 Changed 10 years ago by stuartm

For the record, this is going to be irrelevant when the OSD is ported to mythui, which should happen before Christmas and be available in the 0.23 release which we're aiming to have done within the first 3 months of 2010.

comment:23 Changed 10 years ago by stuartm

Milestone: 0.220.23

comment:24 Changed 10 years ago by stuartm

Cc: stuartm added
Milestone: 0.23unknown

comment:25 Changed 10 years ago by ido

stuart, while the mythui solution sounds great, I really hope the 1Q2010 is achievable. meanwhile we're talking about an issue which is bugging the Israeli users (and to my surprise there's quite a few) and a solution is present that could even go into 0.21-fixes ! why wait another 6 mo (added to 18 already gone) ? thanks

comment:26 Changed 10 years ago by Tony Arie Kolev <kolevtony@…>

I like to support the Ido's opinion - the fixes I sent was tested by me for 7 months - I am using a patched by me version 24x7 for watching/recording TV only from MythTV interface in my home. Why it is not included in fixes, really? Many people in Israel are not able to compile from sources or to install binaries not included in distribution. They need it now! So what is the problem!?

comment:27 Changed 10 years ago by robertm

Ticket locked: set

TRAC is not a voting system.

comment:28 Changed 10 years ago by stuartm

Component: mythtvMythTV - General
Milestone: unknown0.23
Ticket locked: unset

comment:29 Changed 10 years ago by danielk

Milestone: 0.230.24
Version: unknownhead

If the OSD branch does not get merged in 0.24 I'll apply this to trunk, if it does Qt will handle i18n for us.

comment:30 Changed 10 years ago by robertm

Component: MythTV - GeneralMythTV - User Interface Library

Suspect this will be closeable in the near future, merge should be coming up.

comment:31 Changed 9 years ago by danielk

Resolution: wontfix
Status: newclosed

The MythUI OSD has been merged which allows for addressing this problem without the external dependency.

Note: See TracTickets for help on using tickets.