New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Offscreen rendering enhancements #8839

Merged
merged 24 commits into from May 11, 2017

Conversation

Projects
None yet
4 participants
@brenca
Member

brenca commented Mar 5, 2017

This adds support for webview tags and "popup windows" (select element for example) inside an offscreen window. Also implements a CEF PR that simplifies and possibly speeds up GPU offscreen rendering.

Depends on electron/libchromiumcontent#265 ✔️

Fixes #8599
Might fix #8051 (with the added ResizeLock)

Currently only webviews that are created when the offscreen window is initialized are renderable inside the offscreen windows, I'm not sure how to change the RenderWidgetHostView under a webview tag's webContents, but if I (or someone else) can figure that out, that would allow us to swap in other webContents into an offscreen webview as well as rendering a devTools window inside the offscreen window.

'popup' windows inside offscreen webview tags are still not working.

@brenca brenca requested a review from gerhardberger Mar 5, 2017

@bengotow bengotow added the in progress label Mar 5, 2017

@brenca brenca changed the title from Add support for child and popup windows in offscreen rendering mode to Offscreen rendering enhancements Apr 12, 2017

@brenca brenca changed the title from Offscreen rendering enhancements to [WIP] Offscreen rendering enhancements Apr 12, 2017

@gerhardberger gerhardberger changed the title from [WIP] Offscreen rendering enhancements to Offscreen rendering enhancements Apr 19, 2017

@gerhardberger

This comment has been minimized.

Member

gerhardberger commented Apr 19, 2017

Ready for review! 👀

/cc @kevinsawicki @zcbenz

@zcbenz

Do you mind rebasing the changes on chrome58 branch? It is going to be merged to master soon.

@@ -99,6 +99,7 @@ class NativeImage : public mate::Wrappable<NativeImage> {
#endif
gfx::Image image_;
v8::Isolate* isolate_;

This comment has been minimized.

@zcbenz

zcbenz May 1, 2017

Contributor

WrappableBase already stores isolate, it can be accessed via isolate().

@@ -353,6 +360,9 @@ mate::Handle<NativeImage> NativeImage::Resize(
bool height_set = options.GetInteger("height", &height);
size.SetSize(width, height);
isolate_->AdjustAmountOfExternalAllocatedMemory(

This comment has been minimized.

@zcbenz

zcbenz May 1, 2017

Contributor

The memory used by the resized image should have been added up when creating the new NativeImage.

: image_(image), isolate_(isolate) {
Init(isolate_);
isolate_->AdjustAmountOfExternalAllocatedMemory(
image_.Width() * image_.Height() * 4);

This comment has been minimized.

@zcbenz

zcbenz May 1, 2017

Contributor

It is possible to get the precise memory used by the image:

if (image_.HasRepresentation(gfx::Image::kImageRepSkia)) {
  image_.ToImageSkia()->bitmap()->getSize();
}
@@ -183,6 +183,7 @@ class WebContents : public mate::TrackableObject<WebContents>,
void SetFrameRate(int frame_rate);
int GetFrameRate() const;
void Invalidate();
gfx::Size GetSizeForNewRenderView(content::WebContents*) const override;

This comment has been minimized.

@zcbenz

zcbenz May 1, 2017

Contributor

This does not seem to be used anywhere.

This comment has been minimized.

@brenca

brenca May 2, 2017

Member

This should indeed be removed.

@brenca

This comment has been minimized.

Member

brenca commented May 2, 2017

@zcbenz shall I change the target to the chrome58 branch, or should I leave it like this so it can be pulled in after that is pulled in?

@brenca

This comment has been minimized.

Member

brenca commented May 10, 2017

I've rebased this to the latest commit on chrome58 (and once again on top of master since the chrome58 pr has been pulled in)

@zcbenz

zcbenz requested changes May 11, 2017 edited

When running tests on Windows a crash would happen in renderer process:

 	base.dll!00007ff9d462dd19()	Unknown
 	base.dll!00007ff9d465a750()	Unknown
 	gfx.dll!00007ff9d4cb2504()	Unknown
 	gfx.dll!00007ff9d4cb2a5e()	Unknown
>	electron.exe!atom::api::NativeImage::NativeImage(v8::Isolate * isolate, const base::FilePath & hicon_path) Line 220	C++
 	electron.exe!atom::api::NativeImage::CreateFromPath(v8::Isolate * isolate, const base::FilePath & path) Line 485	C++
 	electron.exe!base::internal::FunctorTraits<mate::Handle<atom::api::NativeImage> (__cdecl*)(v8::Isolate * __ptr64,base::FilePath const & __ptr64),void>::Invoke<v8::Isolate * __ptr64,base::FilePath const & __ptr64>(mate::Handle<atom::api::NativeImage>(*)(v8::Isolate *, const base::FilePath &) function, v8::Isolate * && <args_0>, const base::FilePath & <args_1>) Line 164	C++
 	electron.exe!base::internal::InvokeHelper<0,mate::Handle<atom::api::NativeImage> >::MakeItSo<mate::Handle<atom::api::NativeImage> (__cdecl*const & __ptr64)(v8::Isolate * __ptr64,base::FilePath const & __ptr64),v8::Isolate * __ptr64,base::FilePath const & __ptr64>(mate::Handle<atom::api::NativeImage>(*)(v8::Isolate *, const base::FilePath &) & functor, v8::Isolate * && <args_0>, const base::FilePath & <args_1>) Line 285	C++
 	electron.exe!base::internal::Invoker<base::internal::BindState<mate::Handle<atom::api::NativeImage> (__cdecl*)(v8::Isolate * __ptr64,base::FilePath const & __ptr64)>,mate::Handle<atom::api::NativeImage> __cdecl(v8::Isolate * __ptr64,base::FilePath const & __ptr64)>::RunImpl<mate::Handle<atom::api::NativeImage> (__cdecl*const & __ptr64)(v8::Isolate * __ptr64,base::FilePath const & __ptr64),std::tuple<> const & __ptr64>(mate::Handle<atom::api::NativeImage>(*)(v8::Isolate *, const base::FilePath &) & functor, const std::tuple<> & bound, base::IndexSequence<> __formal, v8::Isolate * && <unbound_args_0>, const base::FilePath & <unbound_args_1>) Line 361	C++
 	electron.exe!base::internal::Invoker<base::internal::BindState<mate::Handle<atom::api::NativeImage> (__cdecl*)(v8::Isolate * __ptr64,base::FilePath const & __ptr64)>,mate::Handle<atom::api::NativeImage> __cdecl(v8::Isolate * __ptr64,base::FilePath const & __ptr64)>::Run(base::internal::BindStateBase * base, v8::Isolate * && <unbound_args_0>, const base::FilePath & <unbound_args_1>) Line 339	C++
 	electron.exe!base::internal::RunMixin<base::Callback<mate::Handle<atom::api::NativeImage> __cdecl(v8::Isolate * __ptr64,base::FilePath const & __ptr64),1,1> >::Run(v8::Isolate * <args_0>, const base::FilePath & <args_1>) Line 85	C++
 	electron.exe!mate::internal::Invoker<mate::internal::IndicesHolder<0,1>,v8::Isolate * __ptr64,base::FilePath const & __ptr64>::DispatchToCallback<mate::Handle<atom::api::NativeImage> >(base::Callback<mate::Handle<atom::api::NativeImage> __cdecl(v8::Isolate *,base::FilePath const &),1,1> callback) Line 193	C++
 	electron.exe!mate::internal::Dispatcher<mate::Handle<atom::api::NativeImage> __cdecl(v8::Isolate * __ptr64,base::FilePath const & __ptr64)>::DispatchToCallback(const v8::FunctionCallbackInfo<v8::Value> & info) Line 236	C++
 	v8.dll!00007ff9cdf9be90()	Unknown
 	v8.dll!00007ff9ce080102()	Unknown
 	v8.dll!00007ff9ce0806a1()	Unknown
 	v8.dll!00007ff9ce0804bb()	Unknown
 	[External Code]	
@brenca

This comment has been minimized.

Member

brenca commented May 11, 2017

@zcbenz I've fixed that crash now, the tests seem to be passing.

@zcbenz

zcbenz approved these changes May 11, 2017

@zcbenz zcbenz merged commit 1b1c663 into electron:master May 11, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment