Opened 11 years ago

Closed 9 years ago

Last modified 9 years ago

#2117 closed bug (fixed)

Many translator's about box text is clipped (easy)

Reported by: scottmc Owned by: nobody
Priority: normal Milestone: R1
Component: Add-Ons/Translators Version: R1/pre-alpha1
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: no Platform: All

Description

Affected translators: TGA, TIFF, PPM, RAW, EXR, GIF See attached screenshot for cap of all included translators.

Attachments (8)

screen5.png (216.3 KB ) - added by scottmc 11 years ago.
Screenshot-1.png (150.3 KB ) - added by chico 10 years ago.
after changing codes
T2117.diff (2.3 KB ) - added by chico 10 years ago.
the diff changed the TranslatorWindow.cpp, PPMMain.cpp and GIFTranslator.cpp
2117.diff (39.2 KB ) - added by maxime.simon 10 years ago.
(TIFF, RAW, PPM, EXR, GIF, TGA) These Translators's about box now use the layout manager to be font sensitive.
translator-clips.png (207.7 KB ) - added by scottmc 10 years ago.
jpegTranslator.patch (40.0 KB ) - added by yourpalal 9 years ago.
updated patch: added PopulateInfoFromFormat() method
jpeg2000.patch (39.2 KB ) - added by yourpalal 9 years ago.
use Layout, BaseTranslator TranslatorSettings, etc..
HVIFandSGI.patch (15.0 KB ) - added by yourpalal 9 years ago.
modify HVIF and SGI translators to use the Layout API

Download all attachments as: .zip

Change History (36)

by scottmc, 11 years ago

Attachment: screen5.png added

comment:1 by scottmc, 11 years ago

This might be useful for the PNG Translator box. From: http://www.libpng.org/pub/png/src/libpng-LICENSE.txt

A "png_get_copyright" function is available, for convenient use in "about" boxes and the like:

printf("%s",png_get_copyright(NULL));

Also, the PNG logo (in PNG format, of course) is supplied in the files "pngbar.png" and "pngbar.jpg (88x31) and "pngnow.png" (98x31).

in reply to:  1 comment:2 by scottmc, 11 years ago

Replying to scottmc:

This might be useful for the PNG Translator box. From: http://www.libpng.org/pub/png/src/libpng-LICENSE.txt

A "png_get_copyright" function is available, for convenient use in "about" boxes and the like:

printf("%s",png_get_copyright(NULL));

Also, the PNG logo (in PNG format, of course) is supplied in the files "pngbar.png" and "pngbar.jpg (88x31) and "pngnow.png" (98x31).

Nevermind. I see that's how it was down for PNG already.

comment:3 by scottmc, 11 years ago

Summary: Many translator's about box text is clippedMany translator's about box text is clipped (easy)

by chico, 10 years ago

Attachment: Screenshot-1.png added

after changing codes

by chico, 10 years ago

Attachment: T2117.diff added

the diff changed the TranslatorWindow.cpp, PPMMain.cpp and GIFTranslator.cpp

comment:4 by maxime.simon, 10 years ago

I'm currently working on a solution using the layout API (something font sensitive).

Now the different 'about box' display correctly, but the problem is on the preference panel 'DataTranslations'.
Actually, I know how to differentiate a layout BView from a non-layout BView (using the B_SUPPORTS_LAYOUT flag), but I don't know how I could get the PreferredSize of a layout BView. There are methods to define an ExplicitPreferredSize, but none for the bounds of the layout BView. (This seems normal, but not convenient in this case.)

So if someone has a good idea, it would be welcome. :)

comment:5 by stippi, 10 years ago

The layout-management currently doesn't do very much with the PreferredSize(). You may have to use SetExplicit[Min|Max]Size() to get the behavior you want. Does that help?

comment:6 by maxime.simon, 10 years ago

Indeed, using SetExplicitMinSize() instead of SetExplicitPreferredSize() works a bit better.
Thanks[[BR]] But my problem is mostly that using the method Bounds() on a BView which has a layout give an 'empty' size. (To determine the size of the Translator given to the DataTranslation window we need the size of the Translator BView.)

That's laborious to mix layout and non-layout "BViews". :(

comment:7 by maxime.simon, 10 years ago

Besides, using SetExplicit...Size() invalidates the layout, so the Translator won't be font sensitive. (And match the "MinSize" to the highest font isn't an acceptable solution.)

So the best way I have found to resolve the problem of the DataTranslation window is to rewrite it using the layout API. But could it take part of this ticket?

comment:8 by maxime.simon, 10 years ago

This patch needs the patch of the ticket #3789 to work correctly in the DataTranslation preference panel.

comment:9 by stippi, 10 years ago

I've been playing around with this patch, and it's obviously a lot of work and highly appreciated. Unfortunately, "older" applications don't work so well with layouted translator config views. For example, open WonderBrush, go to File->Export As and chose the GIF Translator. The view just doesn't get any size, since the window that WonderBrush embeds the config view into is not layout-managed. Maybe the problem can be avoided at the config view base class. In AttachedToWindow(), one could check if the parent window has the B_AUTO_UPDATE_SIZE_LIMITS flag and then resize oneself to the computed layout size. I am pretty sure that WonderBrush uses ResizeToPreferred() or GetPreferredSize() to determine the size of the config view. Another option would be to solve this directly in BView::GetPreferredSize(). Right now, that method just returns the current size. But if the view has a layout, it could compute the needed size and return that instead.

comment:10 by maxime.simon, 10 years ago

Oh, my mistake, I didn't check if the patch works in other applications than "about box" and DataTranslation. Let me try to improve it.

comment:11 by stippi, 10 years ago

The problem is most likely in BTranslationRoster::MakeConfigurationView(). It needs to setup the rect correctly and already resize the returned view. Thanks for looking into it!

by maxime.simon, 10 years ago

Attachment: 2117.diff added

(TIFF, RAW, PPM, EXR, GIF, TGA) These Translators's about box now use the layout manager to be font sensitive.

comment:12 by maxime.simon, 10 years ago

Actually, I can't resize the returned view in BTranslationRoster::MakeConfigurationView(). I get an error when checking if the view is locked.
What I did is to resize the view in the MakeConfigurationView() method of layouted translator config views, instead of the one in BTranslationRoster.

This patch works with both non-layouted and layouted applications, so no more need of the patch of the ticket #3789, unless we want to have a layouted DataTranslation pref panel. :)

I don't think the StringViews of the layouted translators really in harmony with the others as they are centered, but I don't see how I can make them align left.

by scottmc, 10 years ago

Attachment: translator-clips.png added

comment:13 by scottmc, 10 years ago

I've attached a screenshot from hrev30665, showing the current status of this ticket. GIF, TGA, RAW, WBI, EXR, TIFF, PPM still have some level of clipping that needs to be addressed. The rest look ok, although there text on the other tabs of the JPEG translator about boxes that is in very tiny print on my 1600x1200 display, also the 2 JPEG ones are the only ones that are resizeable, perhaps they shouldn't be?

comment:14 by leavengood, 10 years ago

Owner: changed from axeld to leavengood
Status: newassigned

I am now looking at Maxime's patch, which definitely improves things for the translators he fixed. But some other translators still look bad at font size 18, and in general I'm annoyed by the inconsistency so I may see about trying to use some common code for at least the translator name, version, authors, etc, sort of like what I did for the screensavers a while ago.

comment:15 by leavengood, 10 years ago

I applied Maxime's patch in hrev35769, with some small changes. There was also a lot of manual patching involved since this patch was so old. But I think it still improves things a lot.

I want to leave this ticket open a bit longer until a few more issues in the translator's about boxes and settings views are fixed.

comment:16 by yourpalal, 9 years ago

I would like to do the JPEG and JPEG2000 translators, rewriting them to use BaseTranslator, TranslatorSettings and the Layout api. Any objections?

comment:17 by stippi, 9 years ago

Perfect, have a go at it!

comment:18 by yourpalal, 9 years ago

Alright, I've attached my patch for the JPEG translator, it now uses BaseTranslator, TranslatorSettings and the Layout API. Alot of code is in the JPEGTranslator.cpp file, and perhaps these should be split up, to improve maintainability, but I left things as they were in this respect.

comment:19 by stippi, 9 years ago

Patch looks pretty good. You can search for "( " to find some minor coding style violations. In NewConfigView(), how do the settings get passed to the translator? The function takes them as argument, but the translator acquires the settings from its owner. Can you elaborate? Thanks a lot for your work!

comment:20 by yourpalal, 9 years ago

Alright, I've run checkstyle on the sources and fixed a bunch of coding style violations (mostly pointer style) and changed the constructors that took a JPEGTranslor* to take a TranslatorSettings*, I think this makes more sense. As it was before, the settings passed to NewConfigView() are acquired from "this", so the new code is nearly functionally equivalent, but the settings also needed to be Released() before returning from NewConfigView(), which they now are. Thanks for your help stippi!

comment:21 by stippi, 9 years ago

Sorry, forgot mentioning another possible improvement to the patch the first time around: The constants like "gOutputFormatCount" can be calculated by dividing the size of the array by the size of the type. So instead of

	int gInputFormatCount = 2; 

You can write

	const int gInputFormatCount = sizeof(gInputFormats) / sizeof(translation_format);

(Hope this is correct, since I am not sure about the terminating entry of the array.) In any case, it looks like this may save someoen a headache in the future. :-)

comment:22 by yourpalal, 9 years ago

Yes, that is correct, the arrays are not terminated, they are always passed/returned with their length. Thanks again :D

by yourpalal, 9 years ago

Attachment: jpegTranslator.patch added

updated patch: added PopulateInfoFromFormat() method

by yourpalal, 9 years ago

Attachment: jpeg2000.patch added

use Layout, BaseTranslator TranslatorSettings, etc..

comment:23 by stippi, 9 years ago

Applied patch to JPEG Translator in hrev36465, great work, Alex!

comment:24 by stippi, 9 years ago

Owner: changed from leavengood to stippi
Status: assignedin-progress

Applied the JPEG2000 Translator patch in hrev36466. Once more, great work! Thanks a bunch. Were these two the last ones before this ticket could be closed? I don't see any clipped text anymore.

comment:25 by stippi, 9 years ago

Owner: changed from stippi to nobody
Status: in-progressassigned

To answer my own question, the HVIF and SGI Translator remain the only ones with clipped text. Leaving ticket open until those two are fixed as well.

by yourpalal, 9 years ago

Attachment: HVIFandSGI.patch added

modify HVIF and SGI translators to use the Layout API

comment:26 by stippi, 9 years ago

Resolution: fixed
Status: assignedclosed

That was quick, Alex, applied in hrev36501! Thanks a bunch!

comment:27 by yourpalal, 9 years ago

Welcome, and thanks for your quick/helpful reviews!

comment:28 by diver, 9 years ago

Component: - GeneralAdd-Ons/Translators
Note: See TracTickets for help on using tickets.