View Issue Details

IDProjectCategoryView StatusLast Update
0024222LazarusLCLpublic2014-09-30 18:01
ReporterChrisFAssigned ToPaul Ishenin 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Platformi386-win32OSWindowsOS VersionXP
Product Version1.0.8Product Build 
Target VersionFixed in Version1.1 (SVN) 
Summary0024222: Incorrect results given by LoadIcon, LoadCursor and LoadBitmap functions in a Windows environment.
Description
The LoadIcon, LoadCursor and LoadBitmap functions implemented in the graphics.pp unit are not fully compatible with the windows functions having the same names.

This is concerning only the case when the hInstance parameter is set to NULL, in order to get predefined icons, cursors and bitmaps from the windows system. See respectively:
http://msdn.microsoft.com/en-us/library/windows/desktop/ms648072%28v=vs.85%29.aspx
http://msdn.microsoft.com/en-us/library/windows/desktop/dd145033%28v=vs.85%29.aspx
http://msdn.microsoft.com/en-us/library/windows/desktop/ms648391%28v=vs.85%29.aspx

The main problems are:

1/ If you declare 'windows' into your 'uses' clause, depending of the position of the 'windows' add into your 'uses' clause, it may be the windows.loadicon (loadcursor, loadbitmap) function which is called, or instead the graphics.loadicon (loadcursor, loadbitmap) one.

Without anyway to guess which one will be called during at runtime, except by "strong casting" them, as for windows.loadicon(...), for instance.

Furthermore there is absolutely no compilation error when 'windows' is added, though calling the graphics.loadicon (loadcursor, loadbitmap) this way should raised one. For instance:
MyCursorHandle:=LoadCursor(0,IDC_WAIT); // This one is invalid for graphics.loadcursor

According to my own tests (but I guess it may depends), when having 'windows' at the beginning of the 'uses' clause the graphics functions are called, while when having 'windows' at the end of the 'uses' clause, the windows functions are called.

2/ If the graphics functions are called with these kinds of call (i.e. hInstance=NULL, and predefined constant values in the lpResource name), an improper result is returned.

First, the current handle is not retrieved (as far as I can see, only the instance of the running application is valid for the concerned code, i.e. .LoadFromResourceName and .LoadFromResourceID methods).

Second, an exception is raised (see attached capture.png), while a null return value should be returned instead (if no handle is retrieved) in case of the windows api (no exception expected, even if the requested resource is not found).
Steps To Reproduce
-Start a new Lazarus/FPC graphic project (in a windows environment)

-Add 'windows' at the beginning of your 'uses' clause (or alternatively at the end; it mays depend),

-Add a new pushbutton into your form, and put this code for it:
procedure TForm1.Button1Click(Sender: TObject);
var i1: integer;
begin
  i1:=LoadCursor(0,IDC_WAIT);
  if i1=0 then
    Button1.Caption:='Error'
  else
    Button1.Caption:='Is OK';
end;

-Run your project and click on the pushbutton. An exception is raised.


If you still can't get any exception when adding 'windows' at your 'uses' clause, you can still simulate the problem with the following conditions (this is only to simulate the problem explained here; this is not intend to be the problem by itself):

-Don't add 'windows' at your 'uses' clause,

-Put the current code for your pushbutton:
const IDC_WAIT=32514; // because this one is coming with 'windows.pp'

procedure TForm1.Button1Click(Sender: TObject);
var i1: integer;
begin
  i1:=LoadCursor(0,PChar(Longword(IDC_WAIT)));
  if i1=0 then
    Button1.Caption:='Error'
  else
    Button1.Caption:='Is OK';
end;
Additional Information
The problem is that there are 2 kinds of functions with exactly the same name, and not the same behavior; and for the win32/win64 widgetset only.

TagsNo tags attached.
Fixed in Revision41817
LazTarget-
WidgetsetWin32/Win64
Attached Files

Relationships

has duplicate 0021915 closedBart Broersma Loading icon problem 

Activities

ChrisF

2013-04-03 16:31

reporter  

capture.png (10,155 bytes)
capture.png (10,155 bytes)

Anton Kavalenka

2013-04-03 17:10

reporter   ~0066779

graphics.LoadXXXX() uses FindResource() internally which is not suitable for stock resources (handle of module=0)

windows.LoadXXXX() internally lookups for stock resources

IMO graphics.pas layer is not intended to emulate windows behaviour.

ocean

2013-04-03 17:33

reporter   ~0066780

I also reported similar year ago, bug 21915

It broke custom component, no compile error, but runtime crash.

These were added in rev 30007 "for winapi compatibility"

Anton Kavalenka

2013-04-03 18:08

reporter   ~0066782

graphics.pas routines are X-platform.
So they read windows-like resources even for MacOSX and Linux.
What the LoadXXXX(0,...) should do under Un*X - load from UI theme?

ChrisF

2013-04-03 18:58

reporter   ~0066783

Last edited: 2013-04-03 18:59

View 2 revisions

IMHO, the problem is the presence of these routines.

Should they be absent (from the graphics module, I mean) as for Delphi, there wouldn't be any problems. The windows API would be used all the time, and everything would be fine.

It seems that they have been added to the graphics module for a windows "compatibility". Extract of graphics.pp:
// for winapi compatibility
function LoadBitmap(hInstance: THandle; lpBitmapName: PChar): HBitmap;
...

I can see 2 possible solutions:
-remove them from the graphics.pp unit. I guess it could of course break an ascendant compatibility for people already using them (for their own resources),
-add the support of the windows resources (i.e. when hInstance=0), but only for the win32/win64 widgetset. Which is probably not really a perfect solution too.

Of course, it's still possible to do nothing, but I believe it would the worst solution; because having a runtime error in "certain" cases seems quite inappropriate to me.

BTW, if this report is really a duplicate of the one already reported in r21915, please close this one.

ocean

2013-04-03 19:25

reporter   ~0066784

You may keep this "better one" and close the olded as duplicate, where "people" didn't quite understand..

Bart Broersma

2013-04-04 00:47

developer   ~0066793

@ocean: I resolved 0021915 as duplicate. Please close that one.

Juha Manninen

2013-05-04 14:07

developer   ~0067416

Ok, to me it looks like moving the functions to lclintf is the right choice.
It already ports many Windows functions the same way.
See lcl/lclintf.pas, lcl/include/winapih.inc, lcl/include/winapi.inc
A patch for that is welcome.

Paul Ishenin

2013-06-23 14:29

manager   ~0068460

Please test and close if ok.

Issue History

Date Modified Username Field Change
2013-04-03 16:31 ChrisF New Issue
2013-04-03 16:31 ChrisF File Added: capture.png
2013-04-03 17:10 Anton Kavalenka Note Added: 0066779
2013-04-03 17:33 ocean Note Added: 0066780
2013-04-03 18:08 Anton Kavalenka Note Added: 0066782
2013-04-03 18:58 ChrisF Note Added: 0066783
2013-04-03 18:59 ChrisF Note Edited: 0066783 View Revisions
2013-04-03 19:25 ocean Note Added: 0066784
2013-04-04 00:46 Bart Broersma Relationship added has duplicate 0021915
2013-04-04 00:47 Bart Broersma Note Added: 0066793
2013-05-04 14:07 Juha Manninen Note Added: 0067416
2013-05-04 14:13 Juha Manninen Assigned To => Juha Manninen
2013-05-04 14:13 Juha Manninen Status new => assigned
2013-06-23 13:13 Juha Manninen Assigned To Juha Manninen => Paul Ishenin
2013-06-23 14:29 Paul Ishenin Fixed in Revision => 41817
2013-06-23 14:29 Paul Ishenin LazTarget => -
2013-06-23 14:29 Paul Ishenin Note Added: 0068460
2013-06-23 14:29 Paul Ishenin Status assigned => resolved
2013-06-23 14:29 Paul Ishenin Fixed in Version => 1.1 (SVN)
2013-06-23 14:29 Paul Ishenin Resolution open => fixed
2014-09-30 18:01 ChrisF Status resolved => closed