Re: [webkit-dev] Compile error in WebGL2RenderingContext.cpp
Thanks Tim!
I didn’t know about C++ variable length arrays. That makes perfect sense now.
Looks like people have been requesting Visual Studio add support for VLAs;
there are no indications they will be supported.
From: thor...@apple.com
Sent: Thursday, January 17, 2019 3:50 PM
To: Salisbury, Mark
Cc: webkit-dev@lists.webkit.org
Subject: Re: [webkit-dev] Compile error in WebGL2RenderingContext.cpp
On Jan 16, 2019, at 20:56, Salisbury, Mark
mailto:mark.salisb...@hp.com>> wrote:
Hello,
Hello!
I’m working on rebasing our downstream webkit port; I just pulled down recent
webkit changes (since July ’18) and I hit a piece of code that’s failing to
compile in Visual Studio 2017. When I look at the code however, I’m not sure
how it compiles on any compiler, or if it compiles, how it’s safe.
This is from Source/WebCore/html/canvas/WebGL2RenderingContext.cpp -
…
int numValues = 0;
#if USE(OPENGL_ES)
m_context->getInternalformativ(target, internalformat,
GraphicsContext3D::NUM_SAMPLE_COUNTS, 1, );
GC3Dint params[numValues];
m_context->getInternalformativ(target, internalformat, pname, numValues,
params);
#else
// On desktop OpenGL 4.1 or below we must emulate glGetInternalformativ.
Visual Studio doesn’t like it because it won’t instantiate an array without a
const size. On the other hand, even if it compiled, passing an array with 0
size doesn’t make a lot of sense, to receive parameters back, which appears to
be the intent of the code.
Why do you think it would be 0?
numValues is passed as an out arg in the first getInternalformativ call, and is
presumably filled with the value for the key NUM_SAMPLE_COUNTS.
That said, this depends on a C99 feature that is maybe only available as a
compiler extension in C++ and it sounds like isn't supported by the VS compiler
("variable length arrays"). So I'm guessing nobody has built it with VS yet.
The file is associated with WebGL2, but it’s added to the build in the
ENABLE_WEBGL block in WebCore/CMakeLists.txt.
WEBGL2 is off by default (per WebKitFeatures.cmake) so I assume the code could
not be invoked without enabling WEBGL 2.
The contents of this file are guarded with #if ENABLE(WEBGL2); you should
probably figure out why that's turned on if you don't expect it.
Am I looking at unfinished code that magically compiles with gcc/llvm/etc. (non
Visual Studio)?
Thanks,
Mark
___
webkit-dev mailing list
webkit-dev@lists.webkit.org<mailto:webkit-dev@lists.webkit.org>
https://lists.webkit.org/mailman/listinfo/webkit-dev<https://lists.webkit.org/mailman/listinfo/webkit-dev>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev
[webkit-dev] Compile error in WebGL2RenderingContext.cpp
Hello, I'm working on rebasing our downstream webkit port; I just pulled down recent webkit changes (since July '18) and I hit a piece of code that's failing to compile in Visual Studio 2017. When I look at the code however, I'm not sure how it compiles on any compiler, or if it compiles, how it's safe. This is from Source/WebCore/html/canvas/WebGL2RenderingContext.cpp - ... int numValues = 0; #if USE(OPENGL_ES) m_context->getInternalformativ(target, internalformat, GraphicsContext3D::NUM_SAMPLE_COUNTS, 1, ); GC3Dint params[numValues]; m_context->getInternalformativ(target, internalformat, pname, numValues, params); #else // On desktop OpenGL 4.1 or below we must emulate glGetInternalformativ. Visual Studio doesn't like it because it won't instantiate an array without a const size. On the other hand, even if it compiled, passing an array with 0 size doesn't make a lot of sense, to receive parameters back, which appears to be the intent of the code. The file is associated with WebGL2, but it's added to the build in the ENABLE_WEBGL block in WebCore/CMakeLists.txt. WEBGL2 is off by default (per WebKitFeatures.cmake) so I assume the code could not be invoked without enabling WEBGL 2. Am I looking at unfinished code that magically compiles with gcc/llvm/etc. (non Visual Studio)? Thanks, Mark ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] unified sources build + forwarding headers that are copies
"Huh, this seems to be a consequence of our conversion to #pragma once" Absolutely. #ifndef _MY_SPECIAL_HEADER_FILE_H #define _MY_SPECIAL_HEADER_FILE_H ... #endif You could then include this MySpecialHeaderFile.h from all sorts of places and the definitions would only appear to the compiler once. #pragma once says "Include this header file (at this location)" one time only. -Mark -Original Message- From: Konstantin Tokarev [mailto:annu...@yandex.ru] Sent: Friday, November 17, 2017 9:16 PM To: Salisbury, Mark <mark.salisb...@hp.com>; webkit-dev@lists.webkit.org Subject: Re: [webkit-dev] unified sources build + forwarding headers that are copies 14.11.2017, 14:54, "Salisbury, Mark" <mark.salisb...@hp.com>: > Hello, > > I’m building on Windows (WPE platform, perhaps I’m the first to try this!) > and I’m seeing some compile errors that I believe are arising from previously > untested combinations of source files. > > If I understand unified sources build correctly, it uses a large list of > files and then arranges them into groups, 8 files at a time. If I have a > slightly different set of files than is standard, or if someone adds a new > source file, the combinations of files can change. If the same header file > is included twice (and forwarding header file is a copy of the original), > this causes duplicate definitions / compile errors. Huh, this seems to be a consequence of our conversion to #pragma once > > My question is – is there a right / ideal way to fix these conflicts? > > d:\git\webkit-org\source\javascriptcore\runtime\ArrayBufferSharingMode.h is > included twice, the first time, via the real path, the second time, via a > JavaScriptCore forwarding header. Note that this forwarding header is a copy > of the header file, not a pointer/alias to the real header path. > > The code that creates the forwarding header version comes from > D:\git\webkit-org\Source\JavaScriptCore\PlatformWin.cmake (see xcopy around > line 40). > > Do non-windows ports use a more intelligent method (a pointer, not a file > copy) for creating forwarding headers for JavaScriptCore includes? > > Is the windows build the only one to use unified sources currently? > > Source/WebKit/Scripts/generate-forwarding-headers.pl scans .c|.cpp files for > forwarding headers that need to be created and creates a header ‘pointer’ to > the real path, so both paths could be included (forwarding header vs. real) > and there would be no conflict, because ultimately, only the real header is > included. > > So I guess I’m suggesting to use a forwarding header ‘pointer’ instead of > header file xcopy, which would mean running a script that recurses > directories in javascriptcore and creates forwarding headers. That would > mean no duplicate header file name would be allowed in JavaScriptCore – I > don’t think that’s a problem though. (I don’t really like the idea of > grep’ing all webcore to create forwarding headers where needed, grep’ing > webcore gets expensive) . Just wondering if there are other suggestions. > > Thanks, > > Mark > > (Personally I don’t like forwarding headers but that’s another topic… not > sure if the benefits outweigh associated problems) > > D:\git\webkit-org\WebKitBuild\Debug\DerivedSources\WebCore\unified-sources\UnifiedSource180.cpp > > Turned on ‘show includes’, you can see the two ways I get to the same header > file: > > First source file including ArrayBufferSharingMode.h: > > 1>Note: including file: D:\git\webkit-org\Source\WebCore\css/DOMMatrix.cpp > > 1>Note: including file: D:\git\webkit-org\Source\WebCore\config.h > > 1>Note: including file: d:\git\webkit-org\source\webcore\css\DOMMatrix.h > > 1>Note: including file: > d:\git\webkit-org\source\webcore\css\DOMMatrixReadOnly.h > > 1>Note: including file: > d:\git\webkit-org\source\webcore\css\DOMMatrixInit.h > > 1>Note: including file: > d:\git\webkit-org\source\webcore\css\DOMMatrix2DInit.h > > 1>Note: including file: > D:\git\webkit-org\Source\WebCore\bindings\js\ScriptWrappable.h > > 1>Note: including file: > D:\git\webkit-org\Source\JavaScriptCore\heap/Weak.h > > 1>Note: including file: > D:\git\webkit-org\Source\WebCore\platform\graphics\transforms\TransformationMatrix.h > > 1>Note: including file: > D:\git\webkit-org\Source\WebCore\platform\graphics\FloatPoint.h > > 1>Note: including file: > d:\git\webkit-org\source\webcore\platform\graphics\FloatSize.h > > 1>Note: including file: > d:\git\webkit-org\source\webcore\platform\graphics\IntPoint.h > > 1>Note: including file: > d:\git\webkit-org\source\
Re: [webkit-dev] unified sources build + forwarding headers that are copies
Thanks for the replies Alex, Michael, Konstantin. I've submitted a bug with proposed patch here: https://bugs.webkit.org/show_bug.cgi?id=179814 -Mark -Original Message- From: achristen...@apple.com [mailto:achristen...@apple.com] Sent: Tuesday, November 14, 2017 12:25 PM To: Salisbury, Mark <mark.salisb...@hp.com> Cc: webkit-dev@lists.webkit.org Subject: Re: [webkit-dev] unified sources build + forwarding headers that are copies Our CMakeLists.txt have many instances of checks like “if (WIN32)” that assume that if you are running CMake on Windows then you are building for Windows. If you can make these checks work for you without breaking the existing Windows builds, then we would welcome such improvements. ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] TimerWindowWndProc and dangling page instances
Vincent,
I've also embedded WebKit in a .NET application, and I've seen similar issues -
not exactly what you're seeing though.
Maybe some tips I've learned could be of help to you or others:
*Calling DestroyWindow will cause WebView::close() to be called (when
WM_DESTROY is received), but it does not cause WebView's destructor to be
called, that doesn't happen until the RCW holding the last COM reference is
destroyed and the COM ref count goes to 0. This happens on the GC thread (.NET
internals), and deleting core webkit objects from a thread besides the main
thread can lead to undefined behavior. I'd see crashes from concurrent
javascript GCs. I fixed this by marshaling the delete call back to the main
thread (in every single COM exposed class).
*WebView's destructor calls DestroyWindow() on a tool tip window which
may or may not have been destroyed (at least on WinCE). When embedding in a
.NET application, there can be a decent gap between the time the WebView's
window is destroyed and the destructor is called. It's possible that in the
instances when Windows destroyed the tool tip window (because it is a child of
the webview window) that the tool tip handle is assigned to another window by
the time WebView's destructor is called. You can guess what happens then when
WebView's destructor deletes the tool tip window. Windows in the system
disappear - sometimes causing a crash. The only fix I could figure out what to
check to see if the window is still a tool tip window before deleting it.
I don't know if anyone wants to see a patch land for either of these problems,
they're confined to embedded WebKit on Windows under .NET...
Mark
From: webkit-dev-boun...@lists.webkit.org
[mailto:webkit-dev-boun...@lists.webkit.org] On Behalf Of Van Den Berghe,
Vincent
Sent: Friday, September 13, 2013 1:43 AM
To: webkit-dev@lists.webkit.org
Subject: [webkit-dev] TimerWindowWndProc and dangling page instances
Hello,
Background
---
I'm currently embedding WebKit in a .NET application, which required me to
solve a whole set of issues that are not relevant here. However, I've stumbled
upon a scenario which I suspect is *really* problematic (but I can be wrong).
Before patching anything, I'd like to submit this to greater minds for
validation. The version of WebKit to which this applies is irrelevant: any
recent version will do.
Problem
--
When I'm finish with a WebView instance and before I'm releasing the COM
object, I close it by doing a
DestroyWindow(m_viewWindow)
...where m_viewWindow is WebView's view window. Note that calling
WebView::close() would work just as well.
This triggers a sequence of events that ultimately causes the destructor of the
page to be called:
Page::~Page()
This works well, but when stress testing this will crash the WebKit.DLL with
the following stack trace:
WebKit.dll!WebCore::setImageLoadingSettings(WebCore::Page *
page) Line 57 + 0x3 bytes C++
WebKit.dll!WebCore::Settings::imageLoadingSettingsTimerFired(WebCore::TimerWebCore::Settings
* __formal) Line 363 + 0x8 bytes C++
WebKit.dll!WebCore::TimerWebCore::XMLHttpRequestProgressEventThrottle::fired()
Line 114 + 0xb bytes C++
WebKit.dll!WebCore::ThreadTimers::sharedTimerFiredInternal()
Line 132 C++
WebKit.dll!WebCore::TimerWindowWndProc(HWND__ * hWnd, unsigned
int message, unsigned int wParam, long lParam) Line 111 C++
Depending on the managed environment (details are irrelevant for this
discussion), the crash happens either immediately or after a while. Because of
the way I embed WebKit, this usually happens when the second (or third,
forth...) instance is being created.
The visible cause of the crash is the call to
WebKit.dll!WebCore::setImageLoadingSettings(WebCore::Page * page)
... with a page instance for which its destructor has already been executed.
Primary Cause
-
I suspect the primary cause of a crash is the pending timer whose message is
still in the message queue, that is executed just after the destruction of the
page, but before any pending timers are killed. This causes a delayed execution
of a function on a dangling page instance.
The problem doesn't occur when I don't call DestroyWindow or WebView::Close(),
but this leads to unacceptable memory leaks.
Workaround
---
The minimal (but far from elegant) workaround is to add a method to
WebCore::Settings:
void clearPage()
{
m_page = 0;
}
And call this method as the first line of Page's destructor:
Page::~Page()
{
m_settings-clearPage();
m_mainFrame-setView(0);
...
...and finally add a if(page) guard in setImageLoadingSettings like so:
static void setImageLoadingSettings(Page* page)
{
if(page)
for (Frame* frame = page-mainFrame(); frame; frame =
frame-tree().traverseNext()) {
Re: [webkit-dev] compact ICU unicode
Thanks Glenn for the feedback. I rather like how Torch Mobile slimmed it down back in '09 for porting to WinCE, but if the primary concern is architectural purity / long term maintainability -- that argument makes sense and I can see why we don't want to muddy WTF with it. At the same time, ICU IS huge and contains way more functionality than WebKit uses ... perhaps it can be slimmed down sufficiently following standard procedures (http://userguide.icu-project.org/packaging) and have it still work. If anyone has had success doing this for WebKit and wouldn't mind sharing what they did to tune it down, I'd appreciate hearing about it. The way it's built for windows (and shipped with WebKitLibraries.zip) it is rather large - larger than WebKit.dll. Thomas is right about WebKit growing substantially - but I notice recently it has shrunk a few MB. (I presume due to pulling out huge features like memory inspector / shadow DOM that are not unmaintained...) Mark -Original Message- From: Thomas Fletcher [mailto:tho...@cranksoftware.com] Sent: Thursday, June 13, 2013 7:38 PM To: Glenn Adams Cc: Salisbury, Mark; WebKit Development (webkit-dev@lists.webkit.org) Subject: Re: [webkit-dev] compact ICU unicode What if we created a new project, based off of ICU called lilICU .. would the WebKit community then accept an alternative binding to this new library? Not to split hairs, but that is essentially what it seems that we would have to do, create a new library, before the WebKit community becomes interested in ways of trimming down WebKit for embedded devices where the resource impositions of dependent libraries are significant. Having worked at porting WebKit to a variety of embedded platforms over the last five years (most of the work non-recontributable due to lack of interest in esoteric and non-mainstream platforms) the size of a typical WebKit build has grown significantly while the number of tuning options has decreased. Thanks, Thomas Glenn Adams wrote: On Sat, Jun 8, 2013 at 3:15 AM, Salisbury, Mark mark.salisb...@hp.com mailto:mark.salisb...@hp.com wrote: Hello, What would people think about including specific ICU data tables in WTF in order to provide a lightweight (but functional) unicode implementation? FWIW, I'd suggest you port ICU to your platform or if the size is too large, port the portion of it that WK uses, and then use that portion. However, I think the ICU library or even a subset should NOT be added to WTF. On embedded systems the size of ICU is prohibitive. Determining the right way to package it to make it small enough isn't simple either. A patch was reviewed once that attempted to add ICU data tables directly in WTF and there were two concerns: 1) Checking in generated files (https://bugs.webkit.org/show_bug.cgi?id=27305#c8) 2) Questions concerning if the ICU license is compatible with WebCore (https://bugs.webkit.org/show_bug.cgi?id=27305#c9) I believe the patch could be done differently as to not check in generated files. Regarding the second concern, ICU has a very permissive license (http://www.icu-project.org/repos/icu/icu/trunk/license.html). There are three requirements, basically that the copyright and permission notice has to appear with copies of the software. I believe that is already a requirement for distributions of webkit that use ICU. Except for WChar unicode, I believe all webkit builds now use ICU Unicode. This Unicode path could replace WCHAR_UNICODE or be introduced as a third option, call it what you like - BASIC_ICU_UNICODE, ICU_LITE_UNICODE, COMPACT_ICU_UNICODE, etc.. I think it might be valuable for other ports that are size conscious - the up and coming NIX port comes to mind. Thanks, Mark Background: After rebasing my WinCE port of webkit, I ran into an ASSERT in WebCore/platform/text/wchar/TextBreakIteratorWchar.cpp, acquireLineBreakIterator(). I thought I'd be able to easily fix this, since I had already modified how LineBreakIterator works to take prior context into account (on my own branch) and find line break in a stream of non-ASCII characters. However, the WCHAR Unicode implementation is very bare bones and does not even support returning the Unicode character category (http://trac.webkit.org/browser/trunk/Source/WTF/wtf/unicode/wchar/UnicodeWchar.cpp#L35). WCHAR Unicode was originally called WinCE Unicode, then it was properly renamed as it had nothing to do with WinCE. WinCE Unicode originally came in here: https://bugs.webkit.org/show_bug.cgi?id=27305. The reason it was introduced was to save space (filesystem and RAM). ICU, if not packaged very carefully (http://userguide.icu-project.org/packaging), is actually larger than webkit itself
[webkit-dev] compact ICU unicode
Hello, What would people think about including specific ICU data tables in WTF in order to provide a lightweight (but functional) unicode implementation? On embedded systems the size of ICU is prohibitive. Determining the right way to package it to make it small enough isn't simple either. A patch was reviewed once that attempted to add ICU data tables directly in WTF and there were two concerns: 1) Checking in generated files (https://bugs.webkit.org/show_bug.cgi?id=27305#c8) 2) Questions concerning if the ICU license is compatible with WebCore (https://bugs.webkit.org/show_bug.cgi?id=27305#c9) I believe the patch could be done differently as to not check in generated files. Regarding the second concern, ICU has a very permissive license (http://www.icu-project.org/repos/icu/icu/trunk/license.html). There are three requirements, basically that the copyright and permission notice has to appear with copies of the software. I believe that is already a requirement for distributions of webkit that use ICU. Except for WChar unicode, I believe all webkit builds now use ICU Unicode. This Unicode path could replace WCHAR_UNICODE or be introduced as a third option, call it what you like - BASIC_ICU_UNICODE, ICU_LITE_UNICODE, COMPACT_ICU_UNICODE, etc.. I think it might be valuable for other ports that are size conscious - the up and coming NIX port comes to mind. Thanks, Mark Background: After rebasing my WinCE port of webkit, I ran into an ASSERT in WebCore/platform/text/wchar/TextBreakIteratorWchar.cpp, acquireLineBreakIterator(). I thought I'd be able to easily fix this, since I had already modified how LineBreakIterator works to take prior context into account (on my own branch) and find line break in a stream of non-ASCII characters. However, the WCHAR Unicode implementation is very bare bones and does not even support returning the Unicode character category (http://trac.webkit.org/browser/trunk/Source/WTF/wtf/unicode/wchar/UnicodeWchar.cpp#L35). WCHAR Unicode was originally called WinCE Unicode, then it was properly renamed as it had nothing to do with WinCE. WinCE Unicode originally came in here: https://bugs.webkit.org/show_bug.cgi?id=27305. The reason it was introduced was to save space (filesystem and RAM). ICU, if not packaged very carefully (http://userguide.icu-project.org/packaging), is actually larger than webkit itself. On embedded systems, this is a big deal. The original plan with the bug above was to include specific ICU data tables in webkit. I've been compiling WTF with Unicode tables embedded for some time now. I don't believe I've seen many layout test regressions due to using a simplified ICU implementation. ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
[webkit-dev] Buildsystem cleanup
I'm glad to see there is some interest in using CMake to build on windows. Using CMake has made working on Windows easier for me (thank you Patrick!)... somewhere on webkit.org there's a page about ideas for making life on windows easier and I think the CMake build system addresses a number of the concerns. Patrick uploaded a patch (https://bugs.webkit.org/show_bug.cgi?id=72816) that uses a strategy of separate CMake platforms for the Apple Windows build, for Windows Cairo, and for Windows CE. I think there's a lot of duplication in having 3 separate CMake platforms for Windows builds. I put another patch out there which creates a single PlatformWin. Build switches passed to CMake then generate the platform you want (Apple, WinCairo - in the future WinCE). I'm curious for those who are interested in CMake for Windows builds which approach they think makes more sense. Duplication but full isolation or no duplication but more opportunity to collide. Both patches are still works in progress. - Mark Salisbury ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
[webkit-dev] Building WebKit with Visual Studio 2008
Hello, I have Visual Studio 2008 installed and I'd like to compile WebKit without downgrading to Visual Studio 2005. I've fixed a few problems along the way which I would like to get checked back into trunk at some point. Now I'm getting a link error I can get past: fatal error C1047: The object or library file 'D:\webkit\webkit.org\WebKitLibraries\win\lib\WebKitSystemInterface.lib' was created with an older compiler than other objects; rebuild old objects and libraries I think I'm getting this error because I'm compiling with VS 2008, and WebKitSystemInterface.lib was compiled with VS 2005. I tried turning off whole program optimization (/LTCG), but then it failed with this error: ...module compiled with /GL found; restarting link with /LTCG; I checked and WebKitSystemInterface.lib is a versioned object; I don't believe the source code to it is part of the webkit trunk. I see that adac...@apple.com, sfal...@apple.com, and m...@apple.com are the last three SVN users to have checked it in, so at least they have the source code. Here are my questions: 1) Is anyone else compiling WebKit with VS 2008? 2) Why isn't the source code for WebKitLibraries\win\lib\WebKitSystemInterface.lib part of the source tree? It seems like such a small part of the rest of the webkit / win code. Could the source code to this lib become part of the webkit open source tree? If not, can someone at Apple rebuild it for me with VS 2008? Maybe we could have both versions checked in? Any other ideas? 3) Assuming I get this working, any volunteers to review my changes and help me get this back to trunk? Thanks, Mark Salisbury ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
